From d2731907492d04b6a3fe88a1fe526f4ebe5b9a65 Mon Sep 17 00:00:00 2001 From: minhqdao Date: Tue, 16 Nov 2021 11:00:43 +0100 Subject: [PATCH] Implement move_color_to_decoration --- example/all.yaml | 1 + lib/src/rules.dart | 2 + lib/src/rules/move_color_to_decoration.dart | 101 ++++++++++++++++++ .../mock_packages/flutter/lib/material.dart | 1 + .../mock_packages/flutter/lib/painting.dart | 5 + .../flutter/lib/src/material/colors.dart | 9 ++ .../lib/src/painting/box_decoration.dart | 5 + test_data/rules/move_color_to_decoration.dart | 65 +++++++++++ 8 files changed, 189 insertions(+) create mode 100644 lib/src/rules/move_color_to_decoration.dart create mode 100644 test_data/mock_packages/flutter/lib/painting.dart create mode 100644 test_data/mock_packages/flutter/lib/src/material/colors.dart create mode 100644 test_data/mock_packages/flutter/lib/src/painting/box_decoration.dart create mode 100644 test_data/rules/move_color_to_decoration.dart diff --git a/example/all.yaml b/example/all.yaml index 937092667..c229ed7ac 100644 --- a/example/all.yaml +++ b/example/all.yaml @@ -85,6 +85,7 @@ linter: - list_remove_unrelated_type - literal_only_boolean_expressions - missing_whitespace_between_adjacent_strings + - move_color_to_decoration - no_adjacent_strings_in_list - no_default_cases - no_duplicate_case_values diff --git a/lib/src/rules.dart b/lib/src/rules.dart index 8e09d1d1c..80e104a7a 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -87,6 +87,7 @@ import 'rules/lines_longer_than_80_chars.dart'; import 'rules/list_remove_unrelated_type.dart'; import 'rules/literal_only_boolean_expressions.dart'; import 'rules/missing_whitespace_between_adjacent_strings.dart'; +import 'rules/move_color_to_decoration.dart'; import 'rules/no_adjacent_strings_in_list.dart'; import 'rules/no_default_cases.dart'; import 'rules/no_duplicate_case_values.dart'; @@ -288,6 +289,7 @@ void registerLintRules({bool inTestMode = false}) { ..register(ListRemoveUnrelatedType()) ..register(LiteralOnlyBooleanExpressions()) ..register(MissingWhitespaceBetweenAdjacentStrings()) + ..register(MoveColorToDecoration()) ..register(NoAdjacentStringsInList()) ..register(NoDefaultCases()) ..register(NoDuplicateCaseValues()) diff --git a/lib/src/rules/move_color_to_decoration.dart b/lib/src/rules/move_color_to_decoration.dart new file mode 100644 index 000000000..0ad59ce34 --- /dev/null +++ b/lib/src/rules/move_color_to_decoration.dart @@ -0,0 +1,101 @@ +// Copyright (c) 2021, 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/flutter_utils.dart'; + +const _desc = r'Move color to decoration.'; + +const _details = r'''Don't provide `Container` with both non-null `color` and +`decoration`. Place `color` inside `decoration` instead. + +**BAD:** +```dart +Widget buildArea() { + return Container( + color: Colors.black, + decoration: BoxDecoration( + border: Border.all(color: Colors.white), + ), + ); +} +``` + +**GOOD:** +```dart +Widget buildArea() { + return Container( + decoration: BoxDecoration( + border: Border.all(color: Colors.white), + color: Colors.black, + ), + ); +} +``` +'''; + +class MoveColorToDecoration extends LintRule { + MoveColorToDecoration() + : super( + name: 'move_color_to_decoration', + description: _desc, + details: _details, + group: Group.errors); + + @override + void registerNodeProcessors( + NodeLintRegistry registry, LinterContext context) { + var visitor = _Visitor(this); + + registry.addInstanceCreationExpression(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + final LintRule rule; + + _Visitor(this.rule); + + @override + void visitInstanceCreationExpression(InstanceCreationExpression node) { + if (!isExactWidgetTypeContainer(node.staticType)) { + return; + } + + var data = _ArgumentData(node.argumentList); + + if (data.wasPositionalArgumentFound) { + return; + } + + if (data.hasColor && data.hasDecoration) { + rule.reportLint(node.constructorName); + } + } +} + +class _ArgumentData { + var wasPositionalArgumentFound = false; + var hasColor = false; + var hasDecoration = false; + + _ArgumentData(ArgumentList node) { + for (var argument in node.arguments) { + if (argument is! NamedExpression) { + wasPositionalArgumentFound = true; + return; + } + var label = argument.name.label; + if (label.name == 'color' && argument.expression is! NullLiteral) { + hasColor = true; + } else if (label.name == 'decoration' && + argument.expression is! NullLiteral) { + hasDecoration = true; + } + } + } +} diff --git a/test_data/mock_packages/flutter/lib/material.dart b/test_data/mock_packages/flutter/lib/material.dart index 674005eda..ecce12efb 100644 --- a/test_data/mock_packages/flutter/lib/material.dart +++ b/test_data/mock_packages/flutter/lib/material.dart @@ -3,5 +3,6 @@ // found in the LICENSE file. export 'src/material/button.dart'; +export 'src/material/colors.dart'; export 'src/material/scaffold.dart'; export 'widgets.dart'; diff --git a/test_data/mock_packages/flutter/lib/painting.dart b/test_data/mock_packages/flutter/lib/painting.dart new file mode 100644 index 000000000..58158555b --- /dev/null +++ b/test_data/mock_packages/flutter/lib/painting.dart @@ -0,0 +1,5 @@ +// Copyright (c) 2021, 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. + +export 'src/painting/box_decoration.dart'; diff --git a/test_data/mock_packages/flutter/lib/src/material/colors.dart b/test_data/mock_packages/flutter/lib/src/material/colors.dart new file mode 100644 index 000000000..6e924b618 --- /dev/null +++ b/test_data/mock_packages/flutter/lib/src/material/colors.dart @@ -0,0 +1,9 @@ +// Copyright (c) 2021, 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. + +class Colors { + Colors._(); + + static const Color transparent = Color(); +} diff --git a/test_data/mock_packages/flutter/lib/src/painting/box_decoration.dart b/test_data/mock_packages/flutter/lib/src/painting/box_decoration.dart new file mode 100644 index 000000000..881503b90 --- /dev/null +++ b/test_data/mock_packages/flutter/lib/src/painting/box_decoration.dart @@ -0,0 +1,5 @@ +// Copyright (c) 2021, 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. + +class BoxDecoration {} diff --git a/test_data/rules/move_color_to_decoration.dart b/test_data/rules/move_color_to_decoration.dart new file mode 100644 index 000000000..2f6b00b42 --- /dev/null +++ b/test_data/rules/move_color_to_decoration.dart @@ -0,0 +1,65 @@ +// Copyright (c) 2021, 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. + +// test w/ `dart test -N move_color_to_decoration` + +import 'package:flutter/material.dart'; +import 'package:flutter/painting.dart'; +import 'package:flutter/widgets.dart'; + + +Widget onlyColor() { + return Container( // OK + color: Colors.transparent, + ); +} + +Widget onlyDecoration() { + return Container( // OK + decoration: BoxDecoration(), + ); +} + +Widget colorAndDecoration() { + return Container( // LINT + color: Colors.transparent, + decoration: BoxDecoration(), + ); +} + +Widget bothNull() { + return Container( // OK + color: null, + decoration: null, + ); +} + +Widget colorIsNull() { + return Container( // OK + color: null, + decoration: BoxDecoration(), + ); +} + +Widget decorationIsNull() { + return Container( // OK + color: Colors.transparent, + decoration: null, + ); +} + +Widget colorWithChild() { + return Container( // OK + color: Colors.transparent, + child: SizedBox(), + ); +} + +Widget withThirdNonNullArgument() { + return Container( // LINT + color: Colors.transparent, + decoration: BoxDecoration(), + child: SizedBox(), + ); +}