Skip to content
This repository was archived by the owner on Jul 16, 2023. It is now read-only.

feat: add allow-nullable config option for avoid-returning-widgets #1180

Merged
merged 3 commits into from
Feb 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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),
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) ?? [];
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)));
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// ignore_for_file: avoid_positional_boolean_parameters

part of 'avoid_returning_widgets_rule.dart';

class _Visitor extends RecursiveAstVisitor<void> {
Expand All @@ -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) {
Expand All @@ -27,6 +30,7 @@ class _Visitor extends RecursiveAstVisitor<void> {
_ignoredNames,
_ignoredAnnotations,
isSetter: node.isSetter,
allowNullable: _allowNullable,
);

if (declaration != null) {
Expand All @@ -44,6 +48,7 @@ class _Visitor extends RecursiveAstVisitor<void> {
final declarationsVisitor = _DeclarationsVisitor(
_ignoredNames,
_ignoredAnnotations,
_allowNullable,
);
node.visitChildren(declarationsVisitor);

Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -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);
}
}

Expand All @@ -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) {
Expand All @@ -121,6 +132,7 @@ class _DeclarationsVisitor extends RecursiveAstVisitor<void> {
_ignoredNames,
_ignoredAnnotations,
isSetter: node.isSetter,
allowNullable: _allowNullable,
);

if (declaration != null) {
Expand All @@ -143,6 +155,7 @@ class _DeclarationsVisitor extends RecursiveAstVisitor<void> {
_ignoredNames,
_ignoredAnnotations,
isSetter: node.isSetter,
allowNullable: _allowNullable,
);

if (declaration != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,5 @@ class _Visitor extends RecursiveAstVisitor<void> {
}

bool _isTypeBoolean(DartType? type) =>
type != null &&
type.isDartCoreBool &&
type.nullabilitySuffix == NullabilitySuffix.none;
type != null && type.isDartCoreBool && !isNullableType(type);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

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';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
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 '../../utils/dart_types_utils.dart';
import '../../utils/node_utils.dart';
import '../../utils/suppression.dart';

Expand Down Expand Up @@ -75,7 +75,7 @@ class DeclarationsVisitor extends RecursiveAstVisitor<void> {
final type = parameter.declaredElement?.type;

return type != null &&
(type.nullabilitySuffix == NullabilitySuffix.question &&
(isNullableType(type) &&
(!parameter.isOptional ||
parameter.isOptional && parameter.isRequired)) ||
(parameter is DefaultFormalParameter &&
Expand Down
Loading