diff --git a/docs/errors/E0716.md b/docs/errors/E0716.md new file mode 100644 index 0000000000..249d3ce47b --- /dev/null +++ b/docs/errors/E0716.md @@ -0,0 +1,25 @@ +# E0716: unintuitive operator precedence when using & and << or >> + +```config-for-examples +{ + "globals": { + "another_variable": true + } +} +``` + +JavaScript's operator precedence can cause confusing situations and bugs when it +comes to bitshifting with a bitwise and. +```js +let a_bitwise_variable = another_variable & 0x1 << 0x2 +``` +The code above looks like it evaluates `another_variable & 0x1` first, +but it instead evaluates `0x1 << 0x2` first. +If this is intended behavior, it may be easier to read if you write parentheses: +```js +another_variable & (0x1 << 0x2) +``` +Otherwise, to get the order of operations correct, add parentheses around the operands of `&`: +```js +(another_variable & 0x1) << 0x2 +``` diff --git a/po/messages.pot b/po/messages.pot index 57504375ed..0d21adfd3b 100644 --- a/po/messages.pot +++ b/po/messages.pot @@ -2089,6 +2089,14 @@ msgstr "" msgid "export default previously appeared here" msgstr "" +#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +msgid "unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &" +msgstr "" + +#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +msgid "'&' here" +msgstr "" + #: test/test-diagnostic-formatter.cpp #: test/test-vim-qflist-json-diag-reporter.cpp msgid "something happened" diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp index 8f4b3cface..1102307596 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp @@ -6425,6 +6425,24 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = { }, }, }, + + // Diag_Unintuitive_Bitshift_Precedence + { + .code = 716, + .severity = Diagnostic_Severity::warning, + .message_formats = { + QLJS_TRANSLATABLE("unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &"), + QLJS_TRANSLATABLE("'&' here"), + }, + .message_args = { + { + Diagnostic_Message_Arg_Info(offsetof(Diag_Unintuitive_Bitshift_Precedence, bitshift_operator), Diagnostic_Arg_Type::source_code_span), + }, + { + Diagnostic_Message_Arg_Info(offsetof(Diag_Unintuitive_Bitshift_Precedence, and_operator), Diagnostic_Arg_Type::source_code_span), + }, + }, + }, }; } diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.h b/src/quick-lint-js/diag/diagnostic-metadata-generated.h index ce26201539..dfeb9eb5c9 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.h +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.h @@ -440,10 +440,11 @@ namespace quick_lint_js { QLJS_DIAG_TYPE_NAME(Diag_Class_Generator_On_Getter_Or_Setter) \ QLJS_DIAG_TYPE_NAME(Diag_Class_Async_On_Getter_Or_Setter) \ QLJS_DIAG_TYPE_NAME(Diag_Multiple_Export_Defaults) \ + QLJS_DIAG_TYPE_NAME(Diag_Unintuitive_Bitshift_Precedence) \ /* END */ // clang-format on -inline constexpr int Diag_Type_Count = 429; +inline constexpr int Diag_Type_Count = 430; extern const Diagnostic_Info all_diagnostic_infos[Diag_Type_Count]; } diff --git a/src/quick-lint-js/diag/diagnostic-types-2.h b/src/quick-lint-js/diag/diagnostic-types-2.h index 59839d2462..c49dcd7cf4 100644 --- a/src/quick-lint-js/diag/diagnostic-types-2.h +++ b/src/quick-lint-js/diag/diagnostic-types-2.h @@ -3318,6 +3318,18 @@ struct Diag_Multiple_Export_Defaults { Source_Code_Span second_export_default; Source_Code_Span first_export_default; }; + +struct Diag_Unintuitive_Bitshift_Precedence { + [[qljs::diag("E0716", Diagnostic_Severity::warning)]] // + // clang-format off + [[qljs::message("unintuitive operator precedence when using & and '{0}'; " + "'{0}' evaluates before &", + ARG(bitshift_operator))]] // + // clang-format on + [[qljs::message("'&' here", ARG(and_operator))]] // + Source_Code_Span bitshift_operator; + Source_Code_Span and_operator; +}; } QLJS_WARNING_POP diff --git a/src/quick-lint-js/fe/parse-expression.cpp b/src/quick-lint-js/fe/parse-expression.cpp index 73c862b5dc..a8ff40c322 100644 --- a/src/quick-lint-js/fe/parse-expression.cpp +++ b/src/quick-lint-js/fe/parse-expression.cpp @@ -73,6 +73,8 @@ void Parser::visit_expression(Expression* ast, Parse_Visitor_Base& v, expression_cast(ast)); this->warn_on_xor_operator_as_exponentiation( expression_cast(ast)); + this->warn_on_unintuitive_bitshift_precedence( + expression_cast(ast)); break; case Expression_Kind::Trailing_Comma: { auto& trailing_comma_ast = diff --git a/src/quick-lint-js/fe/parse.cpp b/src/quick-lint-js/fe/parse.cpp index 90ef2a0ae3..fb7417949b 100644 --- a/src/quick-lint-js/fe/parse.cpp +++ b/src/quick-lint-js/fe/parse.cpp @@ -360,7 +360,24 @@ void Parser::warn_on_comma_operator_in_index(Expression* ast, } } } - +void Parser::warn_on_unintuitive_bitshift_precedence(Expression* ast) { + if (ast->kind() != Expression_Kind::Binary_Operator) return; + if (ast->child_count() <= 2) return; + auto* binary_op = static_cast(ast); + Source_Code_Span left_op = binary_op->operator_spans_[0]; + Source_Code_Span right_op = binary_op->operator_spans_[1]; + if (left_op.string_view() == u8"&"_sv && + (right_op.string_view() == u8">>"_sv || + right_op.string_view() == u8"<<"_sv)) { + if (binary_op->child(0)->kind() == Expression_Kind::Variable && + binary_op->child(1)->kind() == Expression_Kind::Literal && + binary_op->child(2)->kind() == Expression_Kind::Literal) { + this->diag_reporter_->report( + quick_lint_js::Diag_Unintuitive_Bitshift_Precedence{ + .bitshift_operator = right_op, .and_operator = left_op}); + } + } +} void Parser::error_on_pointless_string_compare( Expression::Binary_Operator* ast) { auto is_comparison_operator = [](String8_View s) { diff --git a/src/quick-lint-js/fe/parse.h b/src/quick-lint-js/fe/parse.h index 69d55e4f1c..f3f5f5cdb5 100644 --- a/src/quick-lint-js/fe/parse.h +++ b/src/quick-lint-js/fe/parse.h @@ -526,6 +526,7 @@ class Parser { void warn_on_comma_operator_in_conditional_statement(Expression *); void warn_on_comma_operator_in_index(Expression *, Source_Code_Span); void warn_on_xor_operator_as_exponentiation(Expression::Binary_Operator *); + void warn_on_unintuitive_bitshift_precedence(Expression *ast); void error_on_pointless_string_compare(Expression::Binary_Operator *); void error_on_pointless_compare_against_literal( Expression::Binary_Operator *); diff --git a/src/quick-lint-js/i18n/translation-table-generated.cpp b/src/quick-lint-js/i18n/translation-table-generated.cpp index 04ee6b82d4..f99f767822 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.cpp +++ b/src/quick-lint-js/i18n/translation-table-generated.cpp @@ -18,7 +18,8 @@ const Translation_Table translation_data = { {74, 87, 79, 56, 0, 59}, // {71, 80, 60, 58, 0, 52}, // {0, 0, 0, 0, 0, 28}, // - {31, 56, 0, 32, 0, 63}, // + {0, 0, 0, 0, 0, 63}, // + {31, 56, 0, 32, 0, 9}, // {0, 0, 0, 0, 0, 67}, // {0, 0, 0, 70, 0, 26}, // {79, 25, 30, 63, 49593, 66}, // @@ -499,7 +500,8 @@ const Translation_Table translation_data = { {180, 42, 159, 156, 171, 156}, // {0, 0, 0, 0, 0, 65}, // {92, 45, 78, 81, 70, 43}, // - {98, 37, 86, 82, 83, 77}, // + {0, 0, 0, 0, 0, 77}, // + {98, 37, 86, 82, 83, 81}, // {38, 35, 17, 23, 13, 14}, // {38, 27, 34, 28, 33, 27}, // {26, 41, 26, 32, 0, 22}, // @@ -1813,6 +1815,7 @@ const Translation_Table translation_data = { u8"\"globals\" descriptor must be a boolean or an object\0" u8"\"globals\" must be an object\0" u8"'!' here treated as the TypeScript non-null assertion operator\0" + u8"'&' here\0" u8"'**' operator cannot be used after unary '{1}' without parentheses\0" u8"',' should be ';' instead\0" u8"'.' is not allowed after generic arguments; write [\"{1}\"] instead\0" @@ -2294,6 +2297,7 @@ const Translation_Table translation_data = { u8"unexpected token in variable declaration; expected variable name\0" u8"unexpected whitespace between '!' and '=='\0" u8"unicode byte order mark (BOM) cannot appear before #! at beginning of script\0" + u8"unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &\0" u8"unmatched '}'\0" u8"unmatched indexing bracket\0" u8"unmatched parenthesis\0" diff --git a/src/quick-lint-js/i18n/translation-table-generated.h b/src/quick-lint-js/i18n/translation-table-generated.h index 514e20b711..2819960be4 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.h +++ b/src/quick-lint-js/i18n/translation-table-generated.h @@ -18,8 +18,8 @@ namespace quick_lint_js { using namespace std::literals::string_view_literals; constexpr std::uint32_t translation_table_locale_count = 5; -constexpr std::uint16_t translation_table_mapping_table_size = 524; -constexpr std::size_t translation_table_string_table_size = 80043; +constexpr std::uint16_t translation_table_mapping_table_size = 526; +constexpr std::size_t translation_table_string_table_size = 80133; constexpr std::size_t translation_table_locale_table_size = 35; QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( @@ -33,6 +33,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( "\"globals\" descriptor must be a boolean or an object"sv, "\"globals\" must be an object"sv, "'!' here treated as the TypeScript non-null assertion operator"sv, + "'&' here"sv, "'**' operator cannot be used after unary '{1}' without parentheses"sv, "',' should be ';' instead"sv, "'.' is not allowed after generic arguments; write [\"{1}\"] instead"sv, @@ -514,6 +515,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( "unexpected token in variable declaration; expected variable name"sv, "unexpected whitespace between '!' and '=='"sv, "unicode byte order mark (BOM) cannot appear before #! at beginning of script"sv, + "unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &"sv, "unmatched '}'"sv, "unmatched indexing bracket"sv, "unmatched parenthesis"sv, diff --git a/src/quick-lint-js/i18n/translation-table-test-generated.h b/src/quick-lint-js/i18n/translation-table-test-generated.h index 59c04ad26a..c647722028 100644 --- a/src/quick-lint-js/i18n/translation-table-test-generated.h +++ b/src/quick-lint-js/i18n/translation-table-test-generated.h @@ -27,7 +27,7 @@ struct Translated_String { }; // clang-format off -inline const Translated_String test_translation_table[523] = { +inline const Translated_String test_translation_table[525] = { { "\"global-groups\" entries must be strings"_translatable, u8"\"global-groups\" entries must be strings", @@ -105,6 +105,17 @@ inline const Translated_String test_translation_table[523] = { u8"'!' here treated as the TypeScript non-null assertion operator", }, }, + { + "'&' here"_translatable, + u8"'&' here", + { + u8"'&' here", + u8"'&' here", + u8"'&' here", + u8"'&' here", + u8"'&' here", + }, + }, { "'**' operator cannot be used after unary '{1}' without parentheses"_translatable, u8"'**' operator cannot be used after unary '{1}' without parentheses", @@ -5396,6 +5407,17 @@ inline const Translated_String test_translation_table[523] = { u8"unicode byte ordningsm\u00e4rke (BOM) kan inte f\u00f6rekomma f\u00f6re #! i b\u00f6rjan av skript", }, }, + { + "unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &"_translatable, + u8"unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &", + { + u8"unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &", + u8"unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &", + u8"unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &", + u8"unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &", + u8"unintuitive operator precedence when using & and '{0}'; '{0}' evaluates before &", + }, + }, { "unmatched '}'"_translatable, u8"unmatched '}'", diff --git a/test/test-parse-warning.cpp b/test/test-parse-warning.cpp index 518eff78cc..ea968ec564 100644 --- a/test/test-parse-warning.cpp +++ b/test/test-parse-warning.cpp @@ -436,6 +436,27 @@ TEST_F(Test_Parse_Warning, warn_on_xor_operation_used_as_exponentiation) { test_parse_and_visit_expression(u8"4 ^ 3"_sv, no_diags); test_parse_and_visit_expression(u8"(x+2)^a"_sv, no_diags); } +TEST_F(Test_Parse_Warning, warn_on_unintuitive_precedence_when_using_bitshift) { + test_parse_and_visit_expression( + u8"var1 & 0x01 >> 0x02"_sv, + u8" ^^ Diag_Unintuitive_Bitshift_Precedence.bitshift_operator\n"_diag + u8" ^ .and_operator"_diag); + test_parse_and_visit_expression( + u8"var2 & 0x1234 << 0x4321"_sv, + u8" ^^ Diag_Unintuitive_Bitshift_Precedence.bitshift_operator\n"_diag + u8" ^ .and_operator"_diag); + test_parse_and_visit_statement( + u8"const x = a & 10 << 12"_sv, + u8" ^^ Diag_Unintuitive_Bitshift_Precedence.bitshift_operator\n"_diag + u8" ^ .and_operator"_diag); + test_parse_and_visit_expression(u8"0x111 << 0x222 & var1"_sv, no_diags); + test_parse_and_visit_expression(u8"a&b>>c"_sv, no_diags); + test_parse_and_visit_expression(u8"x & y << z"_sv, no_diags); + test_parse_and_visit_expression(u8"0xABCD & 0xDCBA << 0xFFFF"_sv, no_diags); + test_parse_and_visit_expression(u8"(a & 0o12) >> 0xBEEF"_sv, no_diags); + test_parse_and_visit_expression(u8"a & (b >> 0xBEEF)"_sv, no_diags); + test_parse_and_visit_expression(u8"a & b >> c"_sv, no_diags); +} } }