Skip to content

Commit

Permalink
Add new lints {omit_,specify_non}obvious_property_types
Browse files Browse the repository at this point in the history
See https://github.com/dart-lang/linter/issues/5101 for some background information.

Change-Id: I51b6988f08585b7b4863ae8858d44de831615177
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/387960
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Erik Ernst <[email protected]>
  • Loading branch information
eernstg authored and Commit Queue committed Oct 21, 2024
1 parent d230290 commit e6f81df
Show file tree
Hide file tree
Showing 16 changed files with 1,486 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2198,6 +2198,8 @@ LintCode.omit_local_variable_types:
status: hasFix
LintCode.omit_obvious_local_variable_types:
status: hasFix
LintCode.omit_obvious_property_types:
status: needsEvaluation
LintCode.one_member_abstracts:
status: noFix
notes: |-
Expand Down Expand Up @@ -2359,6 +2361,8 @@ LintCode.sort_unnamed_constructors_first:
status: hasFix
LintCode.specify_nonobvious_local_variable_types:
status: hasFix
LintCode.specify_nonobvious_property_types:
status: needsEvaluation
LintCode.test_types_in_equals:
status: noFix
LintCode.throw_in_finally:
Expand Down
2 changes: 2 additions & 0 deletions pkg/linter/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# 3.7.0-wip

- deprecated lint: `package_api_docs`
- new _(experimental)_ lint: `omit_obvious_property_types`
- new _(experimental)_ lint: `specify_nonobvious_property_types`

# 3.6.0

Expand Down
2 changes: 2 additions & 0 deletions pkg/linter/example/all.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ linter:
- null_closures
- omit_local_variable_types
- omit_obvious_local_variable_types
- omit_obvious_property_types
- one_member_abstracts
- only_throw_errors
- overridden_fields
Expand Down Expand Up @@ -169,6 +170,7 @@ linter:
- sort_pub_dependencies
- sort_unnamed_constructors_first
- specify_nonobvious_local_variable_types
- specify_nonobvious_property_types
- test_types_in_equals
- throw_in_finally
- tighten_type_of_initializing_formals
Expand Down
12 changes: 12 additions & 0 deletions pkg/linter/lib/src/lint_codes.g.dart
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,12 @@ class LinterLintCode extends LintCode {
correctionMessage: "Try removing the type annotation.",
);

static const LintCode omit_obvious_property_types = LinterLintCode(
LintNames.omit_obvious_property_types,
"The type annotation isn't needed because it is obvious.",
correctionMessage: "Try removing the type annotation.",
);

static const LintCode one_member_abstracts = LinterLintCode(
LintNames.one_member_abstracts,
"Unnecessary use of an abstract class.",
Expand Down Expand Up @@ -1481,6 +1487,12 @@ class LinterLintCode extends LintCode {
correctionMessage: "Try adding a type annotation.",
);

static const LintCode specify_nonobvious_property_types = LinterLintCode(
LintNames.specify_nonobvious_property_types,
"A type annotation is needed because it isn't obvious.",
correctionMessage: "Try adding a type annotation.",
);

static const LintCode test_types_in_equals = LinterLintCode(
LintNames.test_types_in_equals,
"Missing type test for '{0}' in '=='.",
Expand Down
6 changes: 6 additions & 0 deletions pkg/linter/lib/src/lint_names.g.dart
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,9 @@ abstract final class LintNames {
static const String omit_obvious_local_variable_types =
'omit_obvious_local_variable_types';

static const String omit_obvious_property_types =
'omit_obvious_property_types';

static const String one_member_abstracts = 'one_member_abstracts';

static const String only_throw_errors = 'only_throw_errors';
Expand Down Expand Up @@ -451,6 +454,9 @@ abstract final class LintNames {
static const String specify_nonobvious_local_variable_types =
'specify_nonobvious_local_variable_types';

static const String specify_nonobvious_property_types =
'specify_nonobvious_property_types';

static const String super_goes_last = 'super_goes_last';

static const String test_types_in_equals = 'test_types_in_equals';
Expand Down
4 changes: 4 additions & 0 deletions pkg/linter/lib/src/rules.dart
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ import 'rules/null_check_on_nullable_type_parameter.dart';
import 'rules/null_closures.dart';
import 'rules/omit_local_variable_types.dart';
import 'rules/omit_obvious_local_variable_types.dart';
import 'rules/omit_obvious_property_types.dart';
import 'rules/one_member_abstracts.dart';
import 'rules/only_throw_errors.dart';
import 'rules/overridden_fields.dart';
Expand Down Expand Up @@ -184,6 +185,7 @@ import 'rules/sort_child_properties_last.dart';
import 'rules/sort_constructors_first.dart';
import 'rules/sort_unnamed_constructors_first.dart';
import 'rules/specify_nonobvious_local_variable_types.dart';
import 'rules/specify_nonobvious_property_types.dart';
import 'rules/super_goes_last.dart';
import 'rules/test_types_in_equals.dart';
import 'rules/throw_in_finally.dart';
Expand Down Expand Up @@ -366,6 +368,7 @@ void registerLintRules() {
..register(NullClosures())
..register(OmitLocalVariableTypes())
..register(OmitObviousLocalVariableTypes())
..register(OmitObviousPropertyTypes())
..register(OneMemberAbstracts())
..register(OnlyThrowErrors())
..register(OverriddenFields())
Expand Down Expand Up @@ -428,6 +431,7 @@ void registerLintRules() {
..register(SortUnnamedConstructorsFirst())
..register(SuperGoesLast())
..register(SpecifyNonObviousLocalVariableTypes())
..register(SpecifyNonObviousPropertyTypes())
..register(TestTypesInEquals())
..register(ThrowInFinally())
..register(TightenTypeOfInitializingFormals())
Expand Down
1 change: 1 addition & 0 deletions pkg/linter/lib/src/rules/always_specify_types.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class AlwaysSpecifyTypes extends LintRule {
LintNames.avoid_types_on_closure_parameters,
LintNames.omit_local_variable_types,
LintNames.omit_obvious_local_variable_types,
LintNames.omit_obvious_property_types,
];

@override
Expand Down
1 change: 1 addition & 0 deletions pkg/linter/lib/src/rules/omit_local_variable_types.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class OmitLocalVariableTypes extends LintRule {
List<String> get incompatibleRules => const [
LintNames.always_specify_types,
LintNames.specify_nonobvious_local_variable_types,
LintNames.specify_nonobvious_property_types,
];

@override
Expand Down
66 changes: 66 additions & 0 deletions pkg/linter/lib/src/rules/omit_obvious_property_types.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// 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 '../analyzer.dart';
import '../util/obvious_types.dart';

const _desc =
r'Omit obvious type annotations for top-level and static variables.';

class OmitObviousPropertyTypes extends LintRule {
OmitObviousPropertyTypes()
: super(
name: 'omit_obvious_property_types',
description: _desc,
state: State.experimental(),
);

@override
List<String> get incompatibleRules => const ['always_specify_types'];

@override
LintCode get lintCode => LinterLintCode.omit_obvious_property_types;

@override
void registerNodeProcessors(
NodeLintRegistry registry, LinterContext context) {
var visitor = _Visitor(this);
registry.addFieldDeclaration(this, visitor);
registry.addTopLevelVariableDeclaration(this, visitor);
}
}

class _Visitor extends SimpleAstVisitor<void> {
final LintRule rule;

_Visitor(this.rule);

@override
void visitFieldDeclaration(FieldDeclaration node) =>
_visitVariableDeclarationList(node.fields);

@override
void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) =>
_visitVariableDeclarationList(node.variables);

void _visitVariableDeclarationList(VariableDeclarationList node) {
var staticType = node.type?.type;
if (staticType == null || staticType.isDartCoreNull) {
return;
}
for (var child in node.variables) {
var initializer = child.initializer;
if (initializer != null && !initializer.hasObviousType) {
return;
}
if (initializer?.staticType != staticType) {
return;
}
}
rule.reportLint(node.type);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class SpecifyNonObviousLocalVariableTypes extends LintRule {
var visitor = _Visitor(this);
registry.addForStatement(this, visitor);
registry.addPatternVariableDeclarationStatement(this, visitor);
registry.addSwitchExpression(this, visitor);
registry.addSwitchStatement(this, visitor);
registry.addVariableDeclarationStatement(this, visitor);
}
Expand Down Expand Up @@ -87,7 +88,10 @@ class _Visitor extends SimpleAstVisitor<void> {
}

@override
void visitSwitchExpression(SwitchExpression node) {}
void visitSwitchExpression(SwitchExpression node) {
if (node.expression.hasObviousType) return;
node.cases.forEach(_PatternVisitor(rule).visitSwitchExpressionCase);
}

@override
void visitSwitchStatement(SwitchStatement node) {
Expand Down
107 changes: 107 additions & 0 deletions pkg/linter/lib/src/rules/specify_nonobvious_property_types.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// 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/element.dart';

import '../analyzer.dart';
import '../util/obvious_types.dart';

const _desc = r'Specify non-obvious type annotations for local variables.';

class SpecifyNonObviousPropertyTypes extends LintRule {
SpecifyNonObviousPropertyTypes()
: super(
name: 'specify_nonobvious_property_types',
description: _desc,
state: State.experimental(),
);

@override
List<String> get incompatibleRules => const ['omit_local_variable_types'];

@override
LintCode get lintCode => LinterLintCode.specify_nonobvious_property_types;

@override
void registerNodeProcessors(
NodeLintRegistry registry, LinterContext context) {
var visitor = _Visitor(this);
registry.addFieldDeclaration(this, visitor);
registry.addTopLevelVariableDeclaration(this, visitor);
}
}

class _Visitor extends SimpleAstVisitor<void> {
final LintRule rule;

_Visitor(this.rule);

@override
void visitFieldDeclaration(FieldDeclaration node) =>
_visitVariableDeclarationList(node.fields,
isInstanceVariable: !node.isStatic);

@override
void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) =>
_visitVariableDeclarationList(node.variables, isInstanceVariable: false);

void _visitVariableDeclarationList(VariableDeclarationList node,
{required bool isInstanceVariable}) {
var staticType = node.type?.type;
if (staticType != null && !staticType.isDartCoreNull) {
return;
}
bool aDeclaredTypeIsNeeded = false;
var variablesThatNeedAType = <VariableDeclaration>[];
for (var child in node.variables) {
var initializer = child.initializer;
if (isInstanceVariable) {
// Ignore this variable if the type comes from override inference.
bool ignoreThisVariable = false;
AstNode? owningDeclaration = node;
while (owningDeclaration != null) {
InterfaceElement? owningElement = switch (owningDeclaration) {
ClassDeclaration(:var declaredElement) => declaredElement,
MixinDeclaration(:var declaredElement) => declaredElement,
EnumDeclaration(:var declaredElement) => declaredElement,
ExtensionTypeDeclaration(:var declaredElement) => declaredElement,
_ => null,
};
if (owningElement != null) {
var variableName = child.name.lexeme;
for (var superInterface in owningElement.allSupertypes) {
if (superInterface.getGetter(variableName) != null) {
ignoreThisVariable = true;
}
if (superInterface.getSetter(variableName) != null) {
ignoreThisVariable = true;
}
}
}
owningDeclaration = owningDeclaration.parent;
}
if (ignoreThisVariable) continue;
}
if (initializer == null) {
aDeclaredTypeIsNeeded = true;
variablesThatNeedAType.add(child);
} else {
if (!initializer.hasObviousType) {
aDeclaredTypeIsNeeded = true;
variablesThatNeedAType.add(child);
}
}
}
if (aDeclaredTypeIsNeeded) {
if (node.variables.length == 1) {
rule.reportLint(node);
} else {
// Multiple variables, report each of them separately. No fix.
variablesThatNeedAType.forEach(rule.reportLint);
}
}
}
}
Loading

0 comments on commit e6f81df

Please sign in to comment.