refactor(fe): mark variables declared inside for loop head

In order to improve some diagnostics, I need to know inside the variable analyzer whether a variable was declared inside the head of a for loop. Add this as a flag.
quick-lint · Sep 23, 2023 · 99d898c · 99d898c
1 parent eebece1
commit 99d898c
Show file tree

Hide file tree

Showing 9 changed files with 167 additions and 30 deletions.
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -32,6 +32,9 @@ Semantic Versioning.
 ### Fixed
 
 * [E0072][] is no longer falsely reported if `function` has a newline after it.
+* [E0196][] is no longer falsely reported if the shadowing variable is declared
+  in the head of a `for` loop. For example, quick-lint-js no longer warns about
+  `let x; for (let x = 0;;);`.
 * TypeScript support (still experimental):
   * A newline after `public`, `protected`, `private`, or `readonly` inside a
     class is now interpreted correctly.

diff --git a/src/quick-lint-js/fe/language.h b/src/quick-lint-js/fe/language.h
@@ -81,6 +81,20 @@ enum Variable_Declaration_Flags : unsigned char {
   // See also NOTE[non-empty-namespace] and
   // Parser::is_current_typescript_namespace_non_empty_.
   non_empty_namespace = 1 << 1,
+
+  // Only valid for _const, _let, and _var.
+  //
+  // Examples set:
+  //   for (let x = 0, y = 42; x < xs.length; ++x);
+  //   for (var [x] of xs);
+  //   for (const key in o);
+  // Examples unset:
+  //   for (x of xs) var y;
+  //   let x;
+  inside_for_loop_head = 1 << 2,
+
+  inside_for_loop_head_initialized_with_equals =
+      inside_for_loop_head | initialized_with_equals,
 };
 
 enum class Function_Attributes {

diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp
@@ -4894,6 +4894,16 @@ void Parser::parse_and_visit_let_bindings(
     });
   }
 
+  Variable_Declaration_Flags flags_without_initializer =
+      options.is_in_for_initializer
+          ? Variable_Declaration_Flags::inside_for_loop_head
+          : Variable_Declaration_Flags::none;
+  Variable_Declaration_Flags flags_with_initializer =
+      options.is_in_for_initializer
+          ? Variable_Declaration_Flags::
+                inside_for_loop_head_initialized_with_equals
+          : Variable_Declaration_Flags::initialized_with_equals;
+
   Source_Code_Span let_span = options.declaring_token.span();
   bool first_binding = true;
   for (;;) {
@@ -5051,7 +5061,7 @@ void Parser::parse_and_visit_let_bindings(
             Binding_Element_Info{
                 .declaration_kind = declaration_kind,
                 .declaring_token = options.declaring_token.span(),
-                .flags = Variable_Declaration_Flags::initialized_with_equals,
+                .flags = flags_with_initializer,
             });
         break;
       }
@@ -5071,7 +5081,7 @@ void Parser::parse_and_visit_let_bindings(
               Binding_Element_Info{
                   .declaration_kind = declaration_kind,
                   .declaring_token = options.declaring_token.span(),
-                  .flags = Variable_Declaration_Flags::none,
+                  .flags = flags_without_initializer,
               });
           this->lexer_.insert_semicolon();
           return;
@@ -5089,8 +5099,7 @@ void Parser::parse_and_visit_let_bindings(
             Binding_Element_Info{
                 .declaration_kind = declaration_kind,
                 .declaring_token = options.declaring_token.span(),
-                // TODO(strager): Would initialized_with_equals make more sense?
-                .flags = Variable_Declaration_Flags::none,
+                .flags = flags_with_initializer,
             });
         break;
       }
@@ -5111,7 +5120,7 @@ void Parser::parse_and_visit_let_bindings(
             Binding_Element_Info{
                 .declaration_kind = declaration_kind,
                 .declaring_token = options.declaring_token.span(),
-                .flags = Variable_Declaration_Flags::none,
+                .flags = flags_without_initializer,
             });
         break;
       }
@@ -5138,7 +5147,7 @@ void Parser::parse_and_visit_let_bindings(
           Binding_Element_Info{
               .declaration_kind = declaration_kind,
               .declaring_token = options.declaring_token.span(),
-              .flags = Variable_Declaration_Flags::none,
+              .flags = flags_without_initializer,
           });
       break;
     }
@@ -5296,7 +5305,9 @@ void Parser::visit_binding_element(Expression *ast, Parse_Visitor_Base &v,
     }
 
     this->visit_expression(rhs, v, Variable_Context::rhs);
-    Variable_Declaration_Flags lhs_flags = Variable_Declaration_Flags::none;
+    Variable_Declaration_Flags lhs_flags =
+        static_cast<Variable_Declaration_Flags>(
+            info.flags & Variable_Declaration_Flags::inside_for_loop_head);
     switch (info.declaration_kind) {
     case Variable_Kind::_const:
     case Variable_Kind::_let:

diff --git a/src/quick-lint-js/fe/variable-analyzer.cpp b/src/quick-lint-js/fe/variable-analyzer.cpp
@@ -754,7 +754,8 @@ void Variable_Analyzer::propagate_variable_declarations_to_parent_scope() {
     }
 
     bool declaration_possibly_looks_like_assignment =
-        (var.flags & Variable_Declaration_Flags::initialized_with_equals);
+        (var.flags & Variable_Declaration_Flags::initialized_with_equals) &&
+        !(var.flags & Variable_Declaration_Flags::inside_for_loop_head);
     if (declaration_possibly_looks_like_assignment && !var.is_used &&
         (var.kind == Variable_Kind::_const ||
          var.kind == Variable_Kind::_let) &&

diff --git a/test/quick-lint-js/spy-visitor.cpp b/test/quick-lint-js/spy-visitor.cpp
@@ -11,6 +11,9 @@ void PrintTo(const Visited_Variable_Declaration &x, std::ostream *out) {
   if (x.flags & Variable_Declaration_Flags::initialized_with_equals) {
     *out << " (initialized with '=')";
   }
+  if (x.flags & Variable_Declaration_Flags::inside_for_loop_head) {
+    *out << " (inside 'for' loop head)";
+  }
   if (x.flags & Variable_Declaration_Flags::non_empty_namespace) {
     *out << " (non-empty)";
   }

diff --git a/test/quick-lint-js/spy-visitor.h b/test/quick-lint-js/spy-visitor.h
@@ -47,8 +47,8 @@ inline Visited_Variable_Declaration class_decl(String8_View name) {
                                       Variable_Declaration_Flags::none};
 }
 
-// A variable declared with 'const' with an initializer. Example: const x =
-// null;
+// A variable declared with 'const' with an initializer.
+// Example: const x = null;
 inline Visited_Variable_Declaration const_init_decl(String8_View name) {
   return Visited_Variable_Declaration{
       String8(name), Variable_Kind::_const,
@@ -62,6 +62,24 @@ inline Visited_Variable_Declaration const_noinit_decl(String8_View name) {
                                       Variable_Declaration_Flags::none};
 }
 
+// A variable declared with 'const' with an initializer in the head of a 'for'
+// loop.
+// Example: for (const length = xs.length; i < length; ++i);
+inline Visited_Variable_Declaration const_init_for_decl(String8_View name) {
+  return Visited_Variable_Declaration{
+      String8(name), Variable_Kind::_const,
+      Variable_Declaration_Flags::inside_for_loop_head_initialized_with_equals};
+}
+
+// A variable declared with 'const' without an initializer in the head of a
+// 'for' loop.
+// Example: for (const x of xs);
+inline Visited_Variable_Declaration const_noinit_for_decl(String8_View name) {
+  return Visited_Variable_Declaration{
+      String8(name), Variable_Kind::_const,
+      Variable_Declaration_Flags::inside_for_loop_head};
+}
+
 inline Visited_Variable_Declaration enum_decl(String8_View name) {
   return Visited_Variable_Declaration{String8(name), Variable_Kind::_enum,
                                       Variable_Declaration_Flags::none};
@@ -137,6 +155,24 @@ inline Visited_Variable_Declaration let_noinit_decl(String8_View name) {
                                       Variable_Declaration_Flags::none};
 }
 
+// A variable declared with 'let' with an initializer in the head of a 'for'
+// loop.
+// Example: for (let x = 0; x < 10; ++x);
+inline Visited_Variable_Declaration let_init_for_decl(String8_View name) {
+  return Visited_Variable_Declaration{
+      String8(name), Variable_Kind::_let,
+      Variable_Declaration_Flags::inside_for_loop_head_initialized_with_equals};
+}
+
+// A variable declared with 'let' without an initializer in the head of a 'for'
+// loop.
+// Example: for (let x of xs);
+inline Visited_Variable_Declaration let_noinit_for_decl(String8_View name) {
+  return Visited_Variable_Declaration{
+      String8(name), Variable_Kind::_let,
+      Variable_Declaration_Flags::inside_for_loop_head};
+}
+
 // A TypeScript namespace (declared with the 'module' or 'namespace' keyword)
 // for which the TypeScript compiler generates JavaScript code.
 inline Visited_Variable_Declaration non_empty_namespace_decl(
@@ -179,6 +215,24 @@ inline Visited_Variable_Declaration var_noinit_decl(String8_View name) {
                                       Variable_Declaration_Flags::none};
 }
 
+// A variable declared with 'var' with an initializer in the head of a 'for'
+// loop.
+// Example: for (var length = xs.length; i < length; ++i);
+inline Visited_Variable_Declaration var_init_for_decl(String8_View name) {
+  return Visited_Variable_Declaration{
+      String8(name), Variable_Kind::_var,
+      Variable_Declaration_Flags::inside_for_loop_head_initialized_with_equals};
+}
+
+// A variable declared with 'var' without an initializer in the head of a
+// 'for' loop.
+// Example: for (var x of xs);
+inline Visited_Variable_Declaration var_noinit_for_decl(String8_View name) {
+  return Visited_Variable_Declaration{
+      String8(name), Variable_Kind::_var,
+      Variable_Declaration_Flags::inside_for_loop_head};
+}
+
 struct Parse_Visit_Collector : public Parse_Visitor_Base {
   std::vector<std::string_view> visits;