Make 'delete x' not warn if x is global

'delete x' is a no-op if the module defines 'x' using 'var', 'let', etc.
However, if 'x' is a global variable, then 'delete x' has side effects.
quick-lint-js reports a warning in both cases, meaning it reports a
false positive in the latter case.

Make quick-lint-js report a warning on 'delete x' only if x is not a
global variable.

Author: strager

src/lint.cpp

@@ -289,6 +289,15 @@ void linter::declare_variable(scope &scope, identifier name, variable_kind kind,
           error_function_call_before_declaration_in_block_scope{used_var.name,
                                                                 name});
     }
+    if (used_var.kind == used_variable_kind::_delete) {
+      // TODO(strager): What if the variable was parenthesized? We should
+      // include the closing parenthesis.
+      this->error_reporter_->report(
+          error_redundant_delete_statement_on_variable{
+              .delete_expression = source_code_span(
+                  used_var.delete_keyword_begin, used_var.name.span().end()),
+          });
+    }
     if (kind == variable_kind::_class || kind == variable_kind::_const ||
         kind == variable_kind::_let) {
       switch (used_var.kind) {
@@ -302,6 +311,9 @@ void linter::declare_variable(scope &scope, identifier name, variable_kind kind,
         this->error_reporter_->report(
             error_variable_used_before_declaration{used_var.name, name});
         break;
+      case used_variable_kind::_delete:
+        // Use before declaration is legal for delete.
+        break;
       case used_variable_kind::_export:
         // Use before declaration is legal for variable exports.
         break;
@@ -324,6 +336,7 @@ void linter::declare_variable(scope &scope, identifier name, variable_kind kind,
                // TODO(strager): This shouldn't happen. export statements are
                // not allowed inside functions.
                break;
+             case used_variable_kind::_delete:
              case used_variable_kind::_typeof:
              case used_variable_kind::use:
                break;
@@ -347,14 +360,23 @@ void linter::visit_variable_assignment(identifier name) {
 
 void linter::visit_variable_delete_use(identifier name,
                                        source_code_span delete_keyword) {
-  this->visit_variable_use(name, used_variable_kind::use);
   QLJS_ASSERT(delete_keyword.end() <= name.span().begin());
-  // TODO(strager): What if the variable was parenthesized? We should include
-  // the closing parenthesis.
-  this->error_reporter_->report(error_redundant_delete_statement_on_variable{
-      .delete_expression =
-          source_code_span(delete_keyword.begin(), name.span().end()),
-  });
+
+  QLJS_ASSERT(!this->scopes_.empty());
+  scope &current_scope = this->current_scope();
+  bool variable_is_declared =
+      current_scope.declared_variables.find(name) != nullptr;
+  if (variable_is_declared) {
+    // TODO(strager): What if the variable was parenthesized? We should include
+    // the closing parenthesis.
+    this->error_reporter_->report(error_redundant_delete_statement_on_variable{
+        .delete_expression =
+            source_code_span(delete_keyword.begin(), name.span().end()),
+    });
+  } else {
+    current_scope.variables_used.emplace_back(name, used_variable_kind::_delete,
+                                              delete_keyword.begin());
+  }
 }
 
 void linter::visit_variable_export_use(identifier name) {
@@ -432,6 +454,10 @@ void linter::visit_end_of_module() {
         this->error_reporter_->report(
             error_assignment_to_undeclared_variable{used_var.name});
         break;
+      case used_variable_kind::_delete:
+        // TODO(strager): Report a warning if the global variable is not
+        // deletable.
+        break;
       case used_variable_kind::_export:
       case used_variable_kind::use:
         this->error_reporter_->report(

src/quick-lint-js/lint.h

@@ -5,6 +5,7 @@
 #define QUICK_LINT_JS_LINT_H
 
 #include <optional>
+#include <quick-lint-js/assert.h>
 #include <quick-lint-js/char8.h>
 #include <quick-lint-js/language.h>
 #include <quick-lint-js/lex.h>
@@ -102,6 +103,7 @@ class linter {
   };
 
   enum class used_variable_kind {
+    _delete,
     _export,
     _typeof,
     assignment,
@@ -110,9 +112,19 @@ class linter {
 
   struct used_variable {
     explicit used_variable(identifier name, used_variable_kind kind) noexcept
-        : name(name), kind(kind) {}
+        : name(name), kind(kind) {
+      QLJS_ASSERT(kind != used_variable_kind::_delete);
+    }
+
+    // kind must be used_variable_kind::_delete.
+    explicit used_variable(identifier name, used_variable_kind kind,
+                           const char8 *delete_keyword_begin) noexcept
+        : name(name), delete_keyword_begin(delete_keyword_begin), kind(kind) {
+      QLJS_ASSERT(kind == used_variable_kind::_delete);
+    }
 
     identifier name;
+    const char8 *delete_keyword_begin;  // used_variable_kind::_delete only
     used_variable_kind kind;
   };
 

test/test-lint.cpp

@@ -2736,6 +2736,87 @@ TEST(test_lint_delete, deleting_local_variable_is_a_warning) {
           offsets_matcher(&delete_expression, 0, u8"delete x"))));
 }
 
+TEST(test_lint_delete, deleting_local_variable_declared_later_is_a_warning) {
+  const char8 declaration[] = u8"v";
+  padded_string delete_expression(u8"delete v"_sv);
+  source_code_span delete_keyword_span(delete_expression.data(),
+                                       delete_expression.data() + 6);
+  ASSERT_EQ(delete_keyword_span.string_view(), u8"delete"_sv);
+  source_code_span deleted_variable_span(delete_expression.data() + 7,
+                                         delete_expression.data() + 8);
+  ASSERT_EQ(deleted_variable_span.string_view(), u8"v"_sv);
+
+  // (() => {
+  //   delete v;
+  //   let v;
+  // });
+  error_collector v;
+  linter l(&v, &default_globals);
+  l.visit_enter_function_scope();
+  l.visit_enter_function_scope_body();
+  l.visit_variable_delete_use(identifier(deleted_variable_span),
+                              delete_keyword_span);
+  l.visit_variable_declaration(identifier_of(declaration), variable_kind::_let);
+  l.visit_exit_function_scope();
+  l.visit_end_of_module();
+
+  EXPECT_THAT(
+      v.errors,
+      ElementsAre(ERROR_TYPE_FIELD(
+          error_redundant_delete_statement_on_variable, delete_expression,
+          offsets_matcher(&delete_expression, 0, u8"delete x"))));
+}
+
+TEST(test_lint_delete, deleting_declared_module_variable_is_a_warning) {
+  const char8 declaration[] = u8"v";
+  padded_string delete_expression(u8"delete v"_sv);
+  source_code_span delete_keyword_span(delete_expression.data(),
+                                       delete_expression.data() + 6);
+  ASSERT_EQ(delete_keyword_span.string_view(), u8"delete"_sv);
+  source_code_span deleted_variable_span(delete_expression.data() + 7,
+                                         delete_expression.data() + 8);
+  ASSERT_EQ(deleted_variable_span.string_view(), u8"v"_sv);
+
+  // let v;
+  // delete v;
+  error_collector v;
+  linter l(&v, &default_globals);
+  l.visit_variable_declaration(identifier_of(declaration), variable_kind::_let);
+  l.visit_variable_delete_use(identifier(deleted_variable_span),
+                              delete_keyword_span);
+  l.visit_end_of_module();
+
+  EXPECT_THAT(
+      v.errors,
+      ElementsAre(ERROR_TYPE_FIELD(
+          error_redundant_delete_statement_on_variable, delete_expression,
+          offsets_matcher(&delete_expression, 0, u8"delete x"))));
+}
+
+TEST(test_lint_delete, deleting_declared_global_variable_is_a_warning) {
+  padded_string code(u8"delete myGlobalVariable"_sv);
+  source_code_span delete_keyword_span(code.data(), code.data() + 6);
+  ASSERT_EQ(delete_keyword_span.string_view(), u8"delete"_sv);
+  source_code_span deleted_variable_span(code.data() + 7, code.data() + 23);
+  ASSERT_EQ(deleted_variable_span.string_view(), u8"myGlobalVariable"_sv);
+
+  configuration config;
+  config.add_global_variable(global_declared_variable{
+      .name = u8"myGlobalVariable",
+      .is_writable = true,
+      .is_shadowable = true,
+  });
+
+  // delete myGlobalVariable;
+  error_collector v;
+  linter l(&v, &config.globals());
+  l.visit_variable_delete_use(identifier(deleted_variable_span),
+                              delete_keyword_span);
+  l.visit_end_of_module();
+
+  EXPECT_THAT(v.errors, IsEmpty());
+}
+
 TEST(test_lint_typeof, using_undeclared_variable_in_typeof_is_not_an_error) {
   const char8 use[] = u8"v";