linter: Remove unsafe_html rule

srawlins · Commit Queue · commit cb5c73e06fba · 2024-11-08T18:31:59.000Z
Fixes https://github.com/dart-lang/linter/issues/5001 Change-Id: I972ea8f9fafda88b6a4836ab92107cebe8a6ad4b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391303 Commit-Queue: Samuel Rawlins <srawlins@google.com> Reviewed-by: Phil Quitslund <pquitslund@google.com>
diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
@@ -2441,12 +2441,6 @@ LintCode.unrelated_type_equality_checks_in_expression:
   status: needsEvaluation
 LintCode.unrelated_type_equality_checks_in_pattern:
   status: needsEvaluation
-LintCode.unsafe_html_attribute:
-  status: noFix
-LintCode.unsafe_html_method:
-  status: noFix
-LintCode.unsafe_html_constructor:
-  status: noFix
 LintCode.use_build_context_synchronously_async_use:
   status: noFix
 LintCode.use_build_context_synchronously_wrong_mounted:
diff --git a/pkg/linter/CHANGELOG.md b/pkg/linter/CHANGELOG.md
@@ -5,6 +5,7 @@
 - _(soon to be)_ deprecated lint: `unsafe_html`
 - new _(experimental)_ lint: `omit_obvious_property_types`
 - new _(experimental)_ lint: `specify_nonobvious_property_types`
+- removed lint: `unsafe_html`
 
 # 3.6.0
 
diff --git a/pkg/linter/example/all.yaml b/pkg/linter/example/all.yaml
@@ -206,7 +206,6 @@ linter:
     - unnecessary_to_list_in_spreads
     - unreachable_from_main
     - unrelated_type_equality_checks
-    - unsafe_html
     - use_build_context_synchronously
     - use_colored_box
     - use_decorated_box
diff --git a/pkg/linter/lib/src/lint_codes.g.dart b/pkg/linter/lib/src/lint_codes.g.dart
@@ -1762,27 +1762,6 @@ class LinterLintCode extends LintCode {
         uniqueName: 'unrelated_type_equality_checks_in_pattern',
       );
 
-  static const LintCode unsafe_html_attribute = LinterLintCode(
-    LintNames.unsafe_html,
-    "Assigning to the attribute '{0}' is unsafe.",
-    correctionMessage: "Try finding a different way to implement the page.",
-    uniqueName: 'unsafe_html_attribute',
-  );
-
-  static const LintCode unsafe_html_constructor = LinterLintCode(
-    LintNames.unsafe_html,
-    "Invoking the constructor '{0}' is unsafe.",
-    correctionMessage: "Try finding a different way to implement the page.",
-    uniqueName: 'unsafe_html_constructor',
-  );
-
-  static const LintCode unsafe_html_method = LinterLintCode(
-    LintNames.unsafe_html,
-    "Invoking the method '{0}' is unsafe.",
-    correctionMessage: "Try finding a different way to implement the page.",
-    uniqueName: 'unsafe_html_method',
-  );
-
   static const LintCode
   use_build_context_synchronously_async_use = LinterLintCode(
     LintNames.use_build_context_synchronously,
diff --git a/pkg/linter/lib/src/rules/unsafe_html.dart b/pkg/linter/lib/src/rules/unsafe_html.dart
@@ -2,173 +2,20 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import 'package:analyzer/dart/ast/ast.dart';
-import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:analyzer/dart/element/element2.dart';
-import 'package:analyzer/dart/element/type.dart';
+import 'package:pub_semver/pub_semver.dart';
 
 import '../analyzer.dart';
-import '../extensions.dart';
 
 const _desc = '$_descPrefix.';
 const _descPrefix = r'Avoid unsafe HTML APIs';
 
 class UnsafeHtml extends LintRule {
   UnsafeHtml()
       : super(
-          name: LintNames.unsafe_html,
-          description: _desc,
-        );
+            name: LintNames.unsafe_html,
+            description: _desc,
+            state: State.removed(since: Version(3, 7, 0)));
 
-  // TODO(brianwilkerson): This lint is not yet using the generated LintCodes.
-  //   We would like to use the codes in the future, but doing
-  //   so requires coordination with other tool teams.
   @override
-  List<LintCode> get lintCodes => [
-        _Visitor.unsafeAttributeCode,
-        _Visitor.unsafeMethodCode,
-        _Visitor.unsafeConstructorCode
-      ];
-
-  @override
-  void registerNodeProcessors(
-      NodeLintRegistry registry, LinterContext context) {
-    var visitor = _Visitor(this);
-    registry.addAssignmentExpression(this, visitor);
-    registry.addInstanceCreationExpression(this, visitor);
-    registry.addMethodInvocation(this, visitor);
-  }
-}
-
-class _Visitor extends SimpleAstVisitor<void> {
-  // TODO(srawlins): Reference attributes ('href', 'src', and 'srcdoc') with
-  // single-quotes to match the convention in the analyzer and linter packages.
-  // This requires some coordination within Google, as various allow-lists are
-  // keyed on the exact text of the LintCode message.
-  // Proposed replacements are commented out in `UnsafeHtml`.
-  static const unsafeAttributeCode = SecurityLintCode(
-    'unsafe_html',
-    '$_descPrefix (assigning "{0}" attribute).',
-    uniqueName: 'LintCode.unsafe_html_attribute',
-  );
-  static const unsafeMethodCode = SecurityLintCode(
-    'unsafe_html',
-    "$_descPrefix (calling the '{0}' method of {1}).",
-    uniqueName: 'LintCode.unsafe_html_method',
-  );
-  static const unsafeConstructorCode = SecurityLintCode(
-    'unsafe_html',
-    "$_descPrefix (calling the '{0}' constructor of {1}).",
-    uniqueName: 'LintCode.unsafe_html_constructor',
-  );
-
-  final LintRule rule;
-
-  _Visitor(this.rule);
-
-  @override
-  void visitAssignmentExpression(AssignmentExpression node) {
-    var leftPart = node.leftHandSide.unParenthesized;
-    if (leftPart is SimpleIdentifier) {
-      var leftPartElement = node.writeElement2;
-      if (leftPartElement == null) return;
-      var enclosingElement = leftPartElement.enclosingElement2;
-      if (enclosingElement is ClassElement2) {
-        _checkAssignment(enclosingElement.thisType, leftPart, node);
-      }
-    } else if (leftPart is PropertyAccess) {
-      _checkAssignment(
-          leftPart.realTarget.staticType, leftPart.propertyName, node);
-    } else if (leftPart is PrefixedIdentifier) {
-      _checkAssignment(leftPart.prefix.staticType, leftPart.identifier, node);
-    }
-  }
-
-  @override
-  void visitInstanceCreationExpression(InstanceCreationExpression node) {
-    var type = node.staticType;
-    if (type == null) return;
-
-    var constructorName = node.constructorName;
-    if (constructorName.name?.name == 'html') {
-      if (type.extendsDartHtmlClass('DocumentFragment')) {
-        rule.reportLint(node,
-            arguments: ['html', 'DocumentFragment'],
-            errorCode: unsafeConstructorCode);
-      } else if (type.extendsDartHtmlClass('Element')) {
-        rule.reportLint(node,
-            arguments: ['html', 'Element'], errorCode: unsafeConstructorCode);
-      }
-    }
-  }
-
-  @override
-  void visitMethodInvocation(MethodInvocation node) {
-    var methodName = node.methodName.name;
-
-    // The static type of the target.
-    DartType? type;
-    if (node.realTarget == null) {
-      // Implicit `this` target.
-      var methodElement = node.methodName.element;
-      if (methodElement == null) return;
-      var enclosingElement = methodElement.enclosingElement2;
-      if (enclosingElement is ClassElement2) {
-        type = enclosingElement.thisType;
-      } else {
-        return;
-      }
-    } else {
-      type = node.realTarget?.staticType;
-      if (type == null) return;
-    }
-
-    if (methodName == 'createFragment' &&
-        (type is DynamicType || type.extendsDartHtmlClass('Element'))) {
-      rule.reportLint(node,
-          arguments: ['createFragment', 'Element'],
-          errorCode: unsafeMethodCode);
-    } else if (methodName == 'setInnerHtml' &&
-        (type is DynamicType || type.extendsDartHtmlClass('Element'))) {
-      rule.reportLint(node,
-          arguments: ['setInnerHtml', 'Element'], errorCode: unsafeMethodCode);
-    } else if (methodName == 'open' &&
-        (type is DynamicType || type.extendsDartHtmlClass('Window'))) {
-      rule.reportLint(node,
-          arguments: ['open', 'Window'], errorCode: unsafeMethodCode);
-    }
-  }
-
-  void _checkAssignment(DartType? type, SimpleIdentifier property,
-      AssignmentExpression assignment) {
-    if (type == null) return;
-
-    // It is more efficient to check the setter's name before checking whether
-    // the target is an interesting type.
-    if (property.name == 'href') {
-      if (type is DynamicType || type.extendsDartHtmlClass('AnchorElement')) {
-        rule.reportLint(assignment,
-            arguments: ['href'], errorCode: unsafeAttributeCode);
-      }
-    } else if (property.name == 'src') {
-      if (type is DynamicType ||
-          type.extendsDartHtmlClass('EmbedElement') ||
-          type.extendsDartHtmlClass('IFrameElement') ||
-          type.extendsDartHtmlClass('ScriptElement')) {
-        rule.reportLint(assignment,
-            arguments: ['src'], errorCode: unsafeAttributeCode);
-      }
-    } else if (property.name == 'srcdoc') {
-      if (type is DynamicType || type.extendsDartHtmlClass('IFrameElement')) {
-        rule.reportLint(assignment,
-            arguments: ['srcdoc'], errorCode: unsafeAttributeCode);
-      }
-    }
-  }
-}
-
-extension on DartType? {
-  /// Returns whether this type extends [className] from the dart:html library.
-  bool extendsDartHtmlClass(String className) =>
-      extendsClass(className, 'dart.dom.html');
+  LintCode get lintCode => LinterLintCode.removed_lint;
 }
diff --git a/pkg/linter/messages.yaml b/pkg/linter/messages.yaml
@@ -12925,6 +12925,7 @@ LintCode:
     addedIn: "2.4"
     categories: [errorProne]
     hasPublishedDocs: false
+    removedIn: "3.7"
     deprecatedDetails: |-
       **NOTE:** This lint is deprecated and will be removed in a future release.
       Remove all inclusions of this lint from your analysis options.
@@ -12946,16 +12947,20 @@ LintCode:
       ```dart
       var script = ScriptElement()..src = 'foo.js';
       ```
+
+      This rule has been removed.
   unsafe_html_constructor:
     sharedName: unsafe_html
     problemMessage: "Invoking the constructor '{0}' is unsafe."
     correctionMessage: "Try finding a different way to implement the page."
     hasPublishedDocs: false
+    removedIn: "3.7"
   unsafe_html_method:
     sharedName: unsafe_html
     problemMessage: "Invoking the method '{0}' is unsafe."
     correctionMessage: "Try finding a different way to implement the page."
     hasPublishedDocs: false
+    removedIn: "3.7"
   use_build_context_synchronously_async_use:
     sharedName: use_build_context_synchronously
     problemMessage: "Don't use 'BuildContext's across async gaps."
diff --git a/pkg/linter/test/all.dart b/pkg/linter/test/all.dart
@@ -15,7 +15,6 @@ import 'mocks.dart';
 import 'pubspec_test.dart' as pubspec;
 import 'rule_test.dart' as rule;
 import 'rules/all.dart' as rules;
-import 'unmocked_sdk_rule_test.dart' as unmocked_sdk_rule;
 import 'utils_test.dart' as utils;
 import 'validate_incompatible_rules_test.dart' as validate_incompatible_rules;
 import 'validate_no_rule_description_references_test.dart'
@@ -41,7 +40,6 @@ void main() {
   pubspec.main();
   rule.main();
   rules.main();
-  unmocked_sdk_rule.main();
   utils.main();
   validate_incompatible_rules.main();
   validate_no_rule_description_references.main();
diff --git a/pkg/linter/test/rules/all.dart b/pkg/linter/test/rules/all.dart
@@ -288,7 +288,6 @@ import 'unnecessary_to_list_in_spreads_test.dart'
 import 'unreachable_from_main_test.dart' as unreachable_from_main;
 import 'unrelated_type_equality_checks_test.dart'
     as unrelated_type_equality_checks;
-import 'unsafe_html_test.dart' as unsafe_html;
 import 'use_build_context_synchronously_test.dart'
     as use_build_context_synchronously;
 import 'use_colored_box_test.dart' as use_colored_box;
@@ -530,7 +529,6 @@ void main() {
   unnecessary_to_list_in_spreads.main();
   unreachable_from_main.main();
   unrelated_type_equality_checks.main();
-  unsafe_html.main();
   use_build_context_synchronously.main();
   use_colored_box.main();
   use_decorated_box.main();
diff --git a/pkg/linter/test/rules/unsafe_html_test.dart b/pkg/linter/test/rules/unsafe_html_test.dart
diff --git a/pkg/linter/test/unmocked_sdk_rule_test.dart b/pkg/linter/test/unmocked_sdk_rule_test.dart
diff --git a/pkg/linter/test_data/rules/unmocked/unsafe_html.dart b/pkg/linter/test_data/rules/unmocked/unsafe_html.dart
diff --git a/pkg/linter/tool/messages_info.dart b/pkg/linter/tool/messages_info.dart
