Merge remote-tracking branch 'origin' into fix-moving-to-variable-false-positives

Author: incendial

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* feat: add `allow-nullable` config option for [`avoid-returning-widgets`](https://dcm.dev/docs/individuals/rules/common/avoid-returning-widgets).
 * fix: support `assert(mounted)` for [`use-setstate-synchronously`](https://dcm.dev/docs/individuals/rules/flutter/use-setstate-synchronously).
 * fix: correctly support dartdoc tags for [`format-comment`](https://dcm.dev/docs/individuals/rules/common/format-comment).
 * fix: resolve several false-positives with while loops, setters and implicit type parameters for [`prefer-moving-to-variable`](https://dcm.dev/docs/individuals/rules/common/prefer-moving-to-variable).

CONTRIBUTING.md

@@ -19,10 +19,8 @@ PR titles should follow the format below:
 To create a new rule:
 
 1. Choose a rule name according to our naming guide or take it from existing issue for the rule.
-2. Add an `.md` file with the rule documentation to `website/docs/rules`. If the rule supports configuration add ![Configurable](https://img.shields.io/badge/-configurable-informational) badge, if it has auto-fixes add ![Has auto-fix](https://img.shields.io/badge/-has%20auto--fix-success) badge.
-3. Add rule entry into rule index file `website/docs/rules/index.mdx`.
-4. Create a rule `.dart` file under `lib/src/analyzers/lint_analyzer/rules/rules_list`.
-5. Create a class that extends an abstract rule class depending on your rule type. Available classes: `FlutterRule`, `CommonRule`, `IntlRule`, `AngularRule`. Add a public field with rule id, documentation url.
+2. Create a rule `.dart` file under `lib/src/analyzers/lint_analyzer/rules/rules_list`.
+3. Create a class that extends an abstract rule class depending on your rule type. Available classes: `FlutterRule`, `CommonRule`, `IntlRule`, `AngularRule`. Add a public field with rule id, documentation url.
 
     The class constructor should take `Map<String, Object> config` parameter which represents config that is passed to the rule from the `analysis_options.yaml`. Example:
 
@@ -36,10 +34,10 @@ To create a new rule:
         );
     ```
 
-6. Add a visitor class which extends any of the base visitors. Usually you will need `RecursiveAstVisitor`. All visitors are [listed there](https://github.com/dart-lang/sdk/blob/master/pkg/analyzer/lib/dart/ast/visitor.dart). Visitor should be added to a separate file and imported with `part` directive.
-7. Add methods overrides to the visitor class for nodes that you want to check (ex. `visitBinaryExpression`, `visitBlock`).
-8. Collect all data needed for the rule (we usually use a private field for data storage and public getter to access it from the `check` method).
-9. In the rule class add override to `check` method. Create a visitor instance and visit all compilation unit children with it.
+4. Add a visitor class which extends any of the base visitors. Usually you will need `RecursiveAstVisitor`. All visitors are [listed there](https://github.com/dart-lang/sdk/blob/master/pkg/analyzer/lib/dart/ast/visitor.dart). Visitor should be added to a separate file and imported with `part` directive.
+5. Add methods overrides to the visitor class for nodes that you want to check (ex. `visitBinaryExpression`, `visitBlock`).
+6. Collect all data needed for the rule (we usually use a private field for data storage and public getter to access it from the `check` method).
+7. In the rule class add override to `check` method. Create a visitor instance and visit all compilation unit children with it.
 
     Convert data to `Issue`'s and return them from the method. Example:
 
@@ -65,7 +63,7 @@ To create a new rule:
     ```
 
     > Override `toJson` method if the rule is configurable.
-10. Add the rule to the `lib/src/analyzers/lint_analyzer/rules/rules_factory.dart`. Example:
+8. Add the rule to the `lib/src/analyzers/lint_analyzer/rules/rules_factory.dart`. Example:
 
     ```dart
     final _implementedRules = <String, Rule Function(Map<String, Object>)>{
@@ -76,8 +74,9 @@ To create a new rule:
     }
     ```
 
-11. Add the rule tests under `test/analyzers/lint_analyzer/rules/rules_list/`. Prefer to split test examples to a correct/incorrect groups.
-12. Add rule into appropriate presets file: `lib/presets/dart_all.yaml` or `lib/presets/flutter_all.yaml`.
+9. Add the rule tests under `test/analyzers/lint_analyzer/rules/rules_list/`. Prefer to split test examples to a correct/incorrect groups.
+10. Add rule into appropriate presets file: `lib/presets/dart_all.yaml` or `lib/presets/flutter_all.yaml`.
+11. Before submitting the PR, add the rule documentation to the PR's description. Refer [to this rule](https://dcm.dev/docs/individuals/rules/common/prefer-trailing-comma/) as an example.
 
 ## Run the plugin in IDE
 

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart

@@ -3,7 +3,6 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element.dart';
-import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:collection/collection.dart';
 

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart

@@ -73,8 +73,7 @@ class _Visitor extends RecursiveAstVisitor<void> {
         childType != null &&
         (_isNotInstance(childType, parentElement) &&
             _isNotDynamic(childType)) &&
-        !(parentElement.type.nullabilitySuffix == NullabilitySuffix.question &&
-            childType.isDartCoreNull)) {
+        !(isNullableType(parentElement.type) && childType.isDartCoreNull)) {
       _expressions.add(node);
     }
   }

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/avoid_redundant_async_rule.dart

@@ -3,8 +3,8 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:analyzer/dart/element/nullability_suffix.dart';
 
+import '../../../../../utils/dart_types_utils.dart';
 import '../../../../../utils/node_utils.dart';
 import '../../../lint_utils.dart';
 import '../../../models/internal_resolved_unit_result.dart';
@@ -35,12 +35,14 @@ class AvoidRedundantAsyncRule extends CommonRule {
 
     source.unit.visitChildren(visitor);
 
-    return visitor.nodes.map(
-      (node) => createIssue(
-        rule: this,
-        location: nodeLocation(node: node, source: source),
-        message: _warningMessage,
-      ),
-    );
+    return visitor.nodes
+        .map(
+          (node) => createIssue(
+            rule: this,
+            location: nodeLocation(node: node, source: source),
+            message: _warningMessage,
+          ),
+        )
+        .toList(growable: false);
   }
 }

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/visitor.dart

@@ -71,9 +71,7 @@ class _AsyncVisitor extends RecursiveAstVisitor<void> {
       _returnsInsideIf.add(node);
     }
 
-    if (type == null ||
-        !type.isDartAsyncFuture ||
-        type.nullabilitySuffix == NullabilitySuffix.question) {
+    if (type == null || !type.isDartAsyncFuture || isNullableType(type)) {
       hasValidAsync = true;
     }
   }

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/avoid_returning_widgets_rule.dart

@@ -5,6 +5,7 @@ import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/type.dart';
 
+import '../../../../../utils/dart_types_utils.dart';
 import '../../../../../utils/flutter_types_utils.dart';
 import '../../../../../utils/node_utils.dart';
 import '../../../lint_utils.dart';
@@ -29,10 +30,12 @@ class AvoidReturningWidgetsRule extends FlutterRule {
 
   final Iterable<String> _ignoredNames;
   final Iterable<String> _ignoredAnnotations;
+  final bool _allowNullable;
 
   AvoidReturningWidgetsRule([Map<String, Object> config = const {}])
       : _ignoredNames = _ConfigParser.getIgnoredNames(config),
         _ignoredAnnotations = _ConfigParser.getIgnoredAnnotations(config),
+        _allowNullable = _ConfigParser.getAllowNullable(config),
         super(
           id: ruleId,
           severity: readSeverity(config, Severity.warning),
@@ -46,13 +49,15 @@ class AvoidReturningWidgetsRule extends FlutterRule {
     json[_ConfigParser._ignoredNamesConfig] = _ignoredNames.toList();
     json[_ConfigParser._ignoredAnnotationsConfig] =
         _ignoredAnnotations.toList();
+    json[_ConfigParser._allowNullable] = _allowNullable;
 
     return json;
   }
 
   @override
   Iterable<Issue> check(InternalResolvedUnitResult source) {
-    final visitor = _Visitor(_ignoredNames, _ignoredAnnotations);
+    final visitor =
+        _Visitor(_ignoredNames, _ignoredAnnotations, _allowNullable);
 
     source.unit.visitChildren(visitor);
 

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/config_parser.dart

@@ -3,6 +3,7 @@ part of 'avoid_returning_widgets_rule.dart';
 class _ConfigParser {
   static const _ignoredNamesConfig = 'ignored-names';
   static const _ignoredAnnotationsConfig = 'ignored-annotations';
+  static const _allowNullable = 'allow-nullable';
 
   static Iterable<String> getIgnoredNames(Map<String, Object> config) =>
       _getIterable(config, _ignoredNamesConfig) ?? [];
@@ -11,6 +12,9 @@ class _ConfigParser {
       _getIterable(config, _ignoredAnnotationsConfig) ??
       functionalWidgetAnnotations;
 
+  static bool getAllowNullable(Map<String, Object> config) =>
+      config[_allowNullable] as bool? ?? false;
+
   static Iterable<String>? _getIterable(
     Map<String, Object> config,
     String name,

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/visit_declaration.dart

@@ -1,24 +1,26 @@
 part of 'avoid_returning_widgets_rule.dart';
 
+// ignore: long-parameter-list
 Declaration? _visitDeclaration(
   Declaration node,
   String name,
   TypeAnnotation? returnType,
   Iterable<String> ignoredNames,
   Iterable<String> ignoredAnnotations, {
   required bool isSetter,
+  required bool allowNullable,
 }) {
   final hasIgnoredAnnotation = node.metadata.any(
     (node) =>
         ignoredAnnotations.contains(node.name.name) &&
         node.atSign.type == TokenType.AT,
   );
 
-  if (!hasIgnoredAnnotation && !isSetter && !_isIgnored(name, ignoredNames)) {
-    final type = returnType?.type;
-    if (type != null && hasWidgetType(type)) {
-      return node;
-    }
+  if (!hasIgnoredAnnotation &&
+      !isSetter &&
+      !_isIgnored(name, ignoredNames) &&
+      _hasWidgetType(returnType?.type, allowNullable)) {
+    return node;
   }
 
   return null;
@@ -29,3 +31,8 @@ bool _isIgnored(
   Iterable<String> ignoredNames,
 ) =>
     name == 'build' || ignoredNames.contains(name);
+
+bool _hasWidgetType(DartType? type, bool allowNullable) =>
+    type != null &&
+    hasWidgetType(type) &&
+    (!allowNullable || (allowNullable && !isNullableType(type)));

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/visitor.dart

@@ -1,3 +1,5 @@
+// ignore_for_file: avoid_positional_boolean_parameters
+
 part of 'avoid_returning_widgets_rule.dart';
 
 class _Visitor extends RecursiveAstVisitor<void> {
@@ -7,12 +9,13 @@ class _Visitor extends RecursiveAstVisitor<void> {
 
   final Iterable<String> _ignoredNames;
   final Iterable<String> _ignoredAnnotations;
+  final bool _allowNullable;
 
   Iterable<InvocationExpression> get invocations => _invocations;
   Iterable<Declaration> get getters => _getters;
   Iterable<Declaration> get globalFunctions => _globalFunctions;
 
-  _Visitor(this._ignoredNames, this._ignoredAnnotations);
+  _Visitor(this._ignoredNames, this._ignoredAnnotations, this._allowNullable);
 
   @override
   void visitFunctionDeclaration(FunctionDeclaration node) {
@@ -27,6 +30,7 @@ class _Visitor extends RecursiveAstVisitor<void> {
       _ignoredNames,
       _ignoredAnnotations,
       isSetter: node.isSetter,
+      allowNullable: _allowNullable,
     );
 
     if (declaration != null) {
@@ -44,6 +48,7 @@ class _Visitor extends RecursiveAstVisitor<void> {
     final declarationsVisitor = _DeclarationsVisitor(
       _ignoredNames,
       _ignoredAnnotations,
+      _allowNullable,
     );
     node.visitChildren(declarationsVisitor);
 
@@ -52,7 +57,7 @@ class _Visitor extends RecursiveAstVisitor<void> {
         .whereType<String>()
         .toSet();
 
-    final invocationsVisitor = _InvocationsVisitor(names);
+    final invocationsVisitor = _InvocationsVisitor(names, _allowNullable);
     node.visitChildren(invocationsVisitor);
 
     _invocations.addAll(invocationsVisitor.invocations);
@@ -64,10 +69,11 @@ class _InvocationsVisitor extends RecursiveAstVisitor<void> {
   final _invocations = <InvocationExpression>[];
 
   final Set<String> _declarationNames;
+  final bool _allowNullable;
 
   Iterable<InvocationExpression> get invocations => _invocations;
 
-  _InvocationsVisitor(this._declarationNames);
+  _InvocationsVisitor(this._declarationNames, this._allowNullable);
 
   @override
   void visitMethodInvocation(MethodInvocation node) {
@@ -90,7 +96,7 @@ class _InvocationsVisitor extends RecursiveAstVisitor<void> {
       final type = expression.staticType;
       if (type is FunctionType) {
         return type.returnType is InterfaceType &&
-            hasWidgetType(type.returnType);
+            _hasWidgetType(type.returnType, _allowNullable);
       }
     }
 
@@ -104,11 +110,16 @@ class _DeclarationsVisitor extends RecursiveAstVisitor<void> {
 
   final Iterable<String> _ignoredNames;
   final Iterable<String> _ignoredAnnotations;
+  final bool _allowNullable;
 
   Iterable<Declaration> get declarations => _declarations;
   Iterable<Declaration> get getters => _getters;
 
-  _DeclarationsVisitor(this._ignoredNames, this._ignoredAnnotations);
+  _DeclarationsVisitor(
+    this._ignoredNames,
+    this._ignoredAnnotations,
+    this._allowNullable,
+  );
 
   @override
   void visitMethodDeclaration(MethodDeclaration node) {
@@ -121,6 +132,7 @@ class _DeclarationsVisitor extends RecursiveAstVisitor<void> {
       _ignoredNames,
       _ignoredAnnotations,
       isSetter: node.isSetter,
+      allowNullable: _allowNullable,
     );
 
     if (declaration != null) {
@@ -143,6 +155,7 @@ class _DeclarationsVisitor extends RecursiveAstVisitor<void> {
       _ignoredNames,
       _ignoredAnnotations,
       isSetter: node.isSetter,
+      allowNullable: _allowNullable,
     );
 
     if (declaration != null) {

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart

@@ -1,6 +1,5 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:collection/collection.dart';
 

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_type_assertions/visitor.dart

@@ -85,10 +85,8 @@ class _Visitor extends RecursiveAstVisitor<void> {
   }
 
   bool _checkNullableCompatibility(DartType objectType, DartType castedType) {
-    final isObjectTypeNullable =
-        objectType.nullabilitySuffix != NullabilitySuffix.none;
-    final isCastedTypeNullable =
-        castedType.nullabilitySuffix != NullabilitySuffix.none;
+    final isObjectTypeNullable = isNullableType(objectType);
+    final isCastedTypeNullable = isNullableType(castedType);
 
     // Only one case `Type? is Type` always valid assertion case.
     return isObjectTypeNullable && !isCastedTypeNullable;

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_rule.dart

@@ -1,9 +1,9 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:collection/collection.dart';
 
+import '../../../../../utils/dart_types_utils.dart';
 import '../../../../../utils/node_utils.dart';
 import '../../../lint_utils.dart';
 import '../../../models/internal_resolved_unit_result.dart';

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_type_casts/visitor.dart

@@ -39,10 +39,8 @@ class _Visitor extends RecursiveAstVisitor<void> {
   }
 
   bool _checkNullableCompatibility(DartType objectType, DartType castedType) {
-    final isObjectTypeNullable =
-        objectType.nullabilitySuffix != NullabilitySuffix.none;
-    final isCastedTypeNullable =
-        castedType.nullabilitySuffix != NullabilitySuffix.none;
+    final isObjectTypeNullable = isNullableType(objectType);
+    final isCastedTypeNullable = isNullableType(castedType);
 
     // Only one case `Type? is Type` always valid assertion case.
     return isObjectTypeNullable && !isCastedTypeNullable;

lib/src/analyzers/lint_analyzer/rules/rules_list/member_ordering/member_ordering_rule.dart

@@ -2,9 +2,9 @@
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:collection/collection.dart';
 
+import '../../../../../utils/dart_types_utils.dart';
 import '../../../../../utils/flutter_types_utils.dart';
 import '../../../../../utils/node_utils.dart';
 import '../../../lint_utils.dart';

lib/src/analyzers/lint_analyzer/rules/rules_list/member_ordering/models/member_group.dart

@@ -43,8 +43,7 @@ class _FieldMemberGroup extends _MemberGroup {
         Identifier.isPrivateName(declaration.fields.variables.first.name.lexeme)
             ? _Modifier.private
             : _Modifier.public;
-    final isNullable = declaration.fields.type?.type?.nullabilitySuffix ==
-        NullabilitySuffix.question;
+    final isNullable = isNullableType(declaration.fields.type?.type);
     final keyword = declaration.fields.isConst
         ? _FieldKeyword.isConst
         : declaration.fields.isFinal

lib/src/analyzers/lint_analyzer/rules/rules_list/no_boolean_literal_compare/no_boolean_literal_compare_rule.dart

@@ -3,9 +3,9 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
 
+import '../../../../../utils/dart_types_utils.dart';
 import '../../../../../utils/node_utils.dart';
 import '../../../lint_utils.dart';
 import '../../../models/internal_resolved_unit_result.dart';