diff --git a/CHANGELOG.md b/CHANGELOG.md index 42143aa9..c681581e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +## 0.1.5 + +- Added `avoid_debug_print` rule +- Fixed an issue with no_magic_number lint +- Fixed `avoid_unused_parameters` to report positional parameters from typedef if their name are not underscores. +- Improvement for `avoid_returning_widget` lint: + - ignores methods that override ones that return widget (build() for example) + - no longer allows returning widgets from methods/functions named build +- Fixed unexpected avoid_unnecessary_type_assertions +- Added `excludeNames` param for `function_lines_of_code` lint +- Improved `avoid_unrelated_type_assertions` to support true and false results + ## 0.1.4 - Removed deprecated lints: @@ -81,12 +93,12 @@ - Update `dart_code_metrics` dependency to 5.7.3 - Rename deprecated `member-ordering-extended` to `member-ordering` - Add rule for widgets methods order configuration: - - initState - - build - - didChangeDependencies - - didUpdateWidget - - deactivate - - dispose + - initState + - build + - didChangeDependencies + - didUpdateWidget + - deactivate + - dispose ## 0.0.15 diff --git a/example/analysis_options.yaml b/example/analysis_options.yaml index c1a963ea..1e39170f 100644 --- a/example/analysis_options.yaml +++ b/example/analysis_options.yaml @@ -19,6 +19,7 @@ custom_lint: - avoid_unnecessary_setstate - double_literal_format - avoid_unnecessary_type_assertions + - avoid_debug_print - avoid_using_api: severity: info entries: diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index 8d5db973..6d1ce562 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -50,6 +50,7 @@ custom_lint: - avoid_unnecessary_type_casts - avoid_unrelated_type_assertions - avoid_unused_parameters + - avoid_debug_print - cyclomatic_complexity: max_complexity: 10 diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index 85efc076..f6524d0b 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -1,6 +1,7 @@ library solid_metrics; import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/src/lints/avoid_debug_print/avoid_debug_print_rule.dart'; import 'package:solid_lints/src/lints/avoid_global_state/avoid_global_state_rule.dart'; import 'package:solid_lints/src/lints/avoid_late_keyword/avoid_late_keyword_rule.dart'; import 'package:solid_lints/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart'; @@ -59,6 +60,7 @@ class _SolidLints extends PluginBase { PreferLastRule.createRule(configs), PreferMatchFileNameRule.createRule(configs), ProperSuperCallsRule.createRule(configs), + AvoidDebugPrint.createRule(configs), ]; // Return only enabled rules diff --git a/lib/src/lints/avoid_debug_print/avoid_debug_print_rule.dart b/lib/src/lints/avoid_debug_print/avoid_debug_print_rule.dart new file mode 100644 index 00000000..73d60c44 --- /dev/null +++ b/lib/src/lints/avoid_debug_print/avoid_debug_print_rule.dart @@ -0,0 +1,125 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/syntactic_entity.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/src/lints/avoid_debug_print/models/avoid_debug_print_func_model.dart'; +import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:solid_lints/src/models/solid_lint_rule.dart'; + +/// A `avoid_debug_print` rule which forbids calling or referencing +/// debugPrint function from flutter/foundation. +/// +/// ### Example +/// +/// #### BAD: +/// +/// ```dart +/// debugPrint(''); // LINT +/// var ref = debugPrint; // LINT +/// var ref2; +/// ref2 = debugPrint; // LINT +/// ``` +/// +/// #### GOOD: +/// +/// ```dart +/// log(''); +/// ``` +class AvoidDebugPrint extends SolidLintRule { + /// The [LintCode] of this lint rule that represents + /// the error when debugPrint is called + static const lintName = 'avoid_debug_print'; + + AvoidDebugPrint._(super.config); + + /// Creates a new instance of [AvoidDebugPrint] + /// based on the lint configuration. + factory AvoidDebugPrint.createRule(CustomLintConfigs configs) { + final rule = RuleConfig( + configs: configs, + name: lintName, + problemMessage: (_) => "Avoid using 'debugPrint'", + ); + + return AvoidDebugPrint._(rule); + } + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addFunctionExpressionInvocation( + (node) { + final func = node.function; + if (func is! Identifier) { + return; + } + _checkIdentifier( + identifier: func, + node: node, + reporter: reporter, + ); + }, + ); + + // addFunctionReference does not get triggered. + // addVariableDeclaration and addAssignmentExpression + // are used as a workaround for simple cases + + context.registry.addVariableDeclaration((node) { + _handleVariableAssignmentDeclaration( + node: node, + reporter: reporter, + ); + }); + + context.registry.addAssignmentExpression((node) { + _handleVariableAssignmentDeclaration( + node: node, + reporter: reporter, + ); + }); + } + + /// Checks whether the function identifier satisfies conditions + void _checkIdentifier({ + required Identifier identifier, + required AstNode node, + required ErrorReporter reporter, + }) { + final funcModel = AvoidDebugPrintFuncModel.parseExpression(identifier); + + if (funcModel.hasSameName && funcModel.hasTheSameSource) { + reporter.reportErrorForNode(code, node); + } + } + + /// Returns null if doesnt have right operand + SyntacticEntity? _getRightOperand(List entities) { + /// Example var t = 15; 15 is in 3d position + if (entities.length < 3) { + return null; + } + return entities[2]; + } + + /// Handles variable assignment and declaration + void _handleVariableAssignmentDeclaration({ + required AstNode node, + required ErrorReporter reporter, + }) { + final rightOperand = _getRightOperand(node.childEntities.toList()); + + if (rightOperand == null || rightOperand is! Identifier) { + return; + } + + _checkIdentifier( + identifier: rightOperand, + node: node, + reporter: reporter, + ); + } +} diff --git a/lib/src/lints/avoid_debug_print/models/avoid_debug_print_func_model.dart b/lib/src/lints/avoid_debug_print/models/avoid_debug_print_func_model.dart new file mode 100644 index 00000000..c0fb542d --- /dev/null +++ b/lib/src/lints/avoid_debug_print/models/avoid_debug_print_func_model.dart @@ -0,0 +1,61 @@ +import 'package:analyzer/dart/ast/ast.dart'; + +/// A class used to parse function expression +class AvoidDebugPrintFuncModel { + /// Function name + final String name; + + /// Function's source path + final String sourcePath; + + /// A class used to parse function expression + const AvoidDebugPrintFuncModel({ + required this.name, + required this.sourcePath, + }); + + /// A constructor that parses identifier into [name] and [sourcePath] + factory AvoidDebugPrintFuncModel.parseExpression( + Identifier identifier, + ) { + switch (identifier) { + case PrefixedIdentifier(): + final prefix = identifier.prefix.name; + return AvoidDebugPrintFuncModel( + name: identifier.name.replaceAll('$prefix.', ''), + sourcePath: + identifier.staticElement?.librarySource?.uri.toString() ?? '', + ); + case SimpleIdentifier(): + return AvoidDebugPrintFuncModel( + name: identifier.name, + sourcePath: + identifier.staticElement?.librarySource?.uri.toString() ?? '', + ); + default: + return AvoidDebugPrintFuncModel._empty(); + } + } + + factory AvoidDebugPrintFuncModel._empty() { + return const AvoidDebugPrintFuncModel( + name: '', + sourcePath: '', + ); + } + + static const String _printPath = 'package:flutter/src/foundation/print.dart'; + + static const String _debugPrint = 'debugPrint'; + + /// Ehether the function has the same source library as debugPrint func + bool get hasTheSameSource => _printPath == sourcePath; + + /// Ehether the function has the same name as debugPrint + bool get hasSameName => _debugPrint == name; + + @override + String toString() { + return '$name, $sourcePath'; + } +} diff --git a/lib/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart b/lib/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart index dcc4f9bc..7d68d009 100644 --- a/lib/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart +++ b/lib/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart @@ -1,4 +1,5 @@ import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/error/listener.dart'; import 'package:custom_lint_builder/custom_lint_builder.dart'; import 'package:solid_lints/src/models/rule_config.dart'; @@ -10,6 +11,9 @@ import 'package:solid_lints/src/utils/types_utils.dart'; /// Using functions instead of Widget subclasses for decomposing Widget trees /// may cause unexpected behavior and performance issues. /// +/// Exceptions: +/// - overriden methods +/// /// More details: https://github.com/flutter/flutter/issues/19269 /// /// ### Example @@ -20,7 +24,8 @@ import 'package:solid_lints/src/utils/types_utils.dart'; /// Widget avoidReturningWidgets() => const SizedBox(); // LINT /// /// class MyWidget extends StatelessWidget { -/// Widget _test1() => const SizedBox(); // LINT +/// Widget get box => SizedBox(); // LINT +/// Widget test1() => const SizedBox(); //LINT /// Widget get _test3 => const SizedBox(); // LINT /// } /// ``` @@ -29,7 +34,14 @@ import 'package:solid_lints/src/utils/types_utils.dart'; /// #### GOOD: /// /// ```dart -/// class MyWidget extends StatelessWidget { +/// class MyWidget extends MyWidget { +/// +/// @override +/// Widget test1() => const SizedBox(); +/// +/// @override +/// Widget get box => ColoredBox(color: Colors.pink); +/// /// @override /// Widget build(BuildContext context) { /// return const SizedBox(); @@ -51,7 +63,8 @@ class AvoidReturningWidgetsRule extends SolidLintRule { name: lintName, problemMessage: (_) => 'Returning a widget from a function is considered an anti-pattern. ' - 'Extract your widget to a separate class.', + 'Unless you are overriding an existing method, ' + 'consider extracting your widget to a separate class.', ); return AvoidReturningWidgetsRule._(rule); @@ -64,15 +77,24 @@ class AvoidReturningWidgetsRule extends SolidLintRule { CustomLintContext context, ) { context.registry.addDeclaration((node) { - final isWidgetReturned = switch (node) { + // Check if declaration is function or method, + // simultaneously checks if return type is [DartType] + final DartType? returnType = switch (node) { FunctionDeclaration(returnType: TypeAnnotation(:final type?)) || MethodDeclaration(returnType: TypeAnnotation(:final type?)) => - hasWidgetType(type), - _ => false, + type, + _ => null, }; - // `build` methods return widgets by nature - if (isWidgetReturned && node.declaredElement?.name != "build") { + if (returnType == null) { + return; + } + + final isWidgetReturned = hasWidgetType(returnType); + + final isOverriden = node.declaredElement?.hasOverride ?? false; + + if (isWidgetReturned && !isOverriden) { reporter.reportErrorForNode(code, node); } }); diff --git a/lib/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart b/lib/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart index c9f2243d..3efef221 100644 --- a/lib/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart +++ b/lib/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart @@ -98,23 +98,12 @@ class AvoidUnnecessaryTypeAssertions extends SolidLintRule { if (objectType == null || castedType == null) { return false; } - final typeCast = TypeCast( source: objectType, target: castedType, - isReversed: true, + isReversed: node.notOperator != null, ); - - if (node.notOperator != null && - objectType is! TypeParameterType && - objectType is! DynamicType && - !objectType.isDartCoreObject && - typeCast.isUnnecessaryTypeCheck) { - return true; - } else { - final typeCast = TypeCast(source: objectType, target: castedType); - return typeCast.isUnnecessaryTypeCheck; - } + return typeCast.isUnnecessaryTypeCheck; } bool _isUnnecessaryWhereType(MethodInvocation node) { diff --git a/lib/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule.dart b/lib/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule.dart index 763a6f17..1d5d4b78 100644 --- a/lib/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule.dart +++ b/lib/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule.dart @@ -22,7 +22,7 @@ class AvoidUnrelatedTypeAssertionsRule extends SolidLintRule { configs: configs, name: lintName, problemMessage: (_) => - 'Avoid unrelated "is" assertion. The result is always "false".', + 'Avoid unrelated "is" assertion. The result is always "{0}".', ); return AvoidUnrelatedTypeAssertionsRule._(rule); @@ -39,7 +39,11 @@ class AvoidUnrelatedTypeAssertionsRule extends SolidLintRule { visitor.visitIsExpression(node); for (final element in visitor.expressions.entries) { - reporter.reportErrorForNode(code, element.key); + reporter.reportErrorForNode( + code, + element.key, + [element.value.toString()], + ); } }); } diff --git a/lib/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_visitor.dart b/lib/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_visitor.dart index 55430aa2..e0b4aa6a 100644 --- a/lib/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_visitor.dart +++ b/lib/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_visitor.dart @@ -30,25 +30,24 @@ import 'package:collection/collection.dart'; /// AST Visitor which finds all is expressions and checks if they are /// unrelated (result always false) class AvoidUnrelatedTypeAssertionsVisitor extends RecursiveAstVisitor { - final _expressions = {}; + final _expressions = {}; - /// All is expressions - Map get expressions => _expressions; + /// Map of unrelated type checks and their results + Map get expressions => _expressions; @override void visitIsExpression(IsExpression node) { super.visitIsExpression(node); final castedType = node.type.type; - if (node.notOperator != null || castedType is TypeParameterType) { + if (castedType is TypeParameterType) { return; } final objectType = node.expression.staticType; if (_isUnrelatedTypeCheck(objectType, castedType)) { - _expressions[node] = - '${node.isOperator.keyword?.lexeme ?? ''}${node.notOperator ?? ''}'; + _expressions[node] = node.notOperator != null; } } diff --git a/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart b/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart index 1770e001..e48a3116 100644 --- a/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart +++ b/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart @@ -9,6 +9,14 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example: /// #### BAD: /// ```dart +/// typedef MaxFun = int Function(int a, int b); +/// final MaxFun bad = (int a, int b) => 1; // LINT +/// final MaxFun testFun = (int a, int b) { // LINT +/// return 4; +/// }; +/// final optional = (int a, [int b = 0]) { // LINT +/// return a; +/// }; /// void fun(String x) {} // LINT /// void fun2(String x, String y) { // LINT /// print(y); @@ -27,6 +35,11 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// /// #### GOOD: /// ```dart +/// typedef MaxFun = int Function(int a, int b); +/// final MaxFun good = (int a, int b) => a + b; +/// final MaxFun testFun = (int a, int b) { +/// return a + b; +/// }; /// void fun(String _) {} // Replacing with underscores silences the warning /// void fun2(String _, String y) { /// print(y); @@ -42,6 +55,15 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// } /// } /// ``` +/// +/// #### Allowed: +/// ```dart +/// typedef Named = String Function({required String text}); +/// final Named named = ({required text}) { +/// return ''; +/// }; +/// +/// ``` class AvoidUnusedParametersRule extends SolidLintRule { /// The [LintCode] of this lint rule that represents /// the error whether we use bad formatted double literals. diff --git a/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_visitor.dart b/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_visitor.dart index 284521cc..40b19f25 100644 --- a/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_visitor.dart +++ b/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_visitor.dart @@ -48,13 +48,14 @@ class AvoidUnusedParametersVisitor extends RecursiveAstVisitor { parameters.parameters.isEmpty) { return; } + final unused = _getUnusedParameters( + node.body, + parameters.parameters, + initializers: node.initializers, + ).whereNot(nameConsistsOfUnderscoresOnly); _unusedParameters.addAll( - _getUnusedParameters( - node.body, - parameters.parameters, - initializers: node.initializers, - ).whereNot(nameConsistsOfUnderscoresOnly), + unused, ); } @@ -76,31 +77,41 @@ class AvoidUnusedParametersVisitor extends RecursiveAstVisitor { if (!isOverride(node.metadata) && !isTearOff) { _unusedParameters.addAll( - _getUnusedParameters( + _filterOutUnderscoresAndNamed( node.body, parameters.parameters, - ).whereNot(nameConsistsOfUnderscoresOnly), + ), ); } } @override - void visitFunctionDeclaration(FunctionDeclaration node) { - super.visitFunctionDeclaration(node); - - final parameters = node.functionExpression.parameters; - - if (node.externalKeyword != null || - (parameters == null || parameters.parameters.isEmpty)) { + void visitFunctionExpression(FunctionExpression node) { + super.visitFunctionExpression(node); + final params = node.parameters; + if (params == null) { return; } _unusedParameters.addAll( - _getUnusedParameters( - node.functionExpression.body, - parameters.parameters, - ).whereNot(nameConsistsOfUnderscoresOnly), + _filterOutUnderscoresAndNamed( + node.body, + params.parameters, + ), + ); + } + + Iterable _filterOutUnderscoresAndNamed( + AstNode body, + Iterable parameters, + ) { + final unused = _getUnusedParameters( + body, + parameters, ); + return unused.whereNot(nameConsistsOfUnderscoresOnly).where( + (param) => !param.isNamed, + ); } Set _getUnusedParameters( @@ -134,6 +145,7 @@ class AvoidUnusedParametersVisitor extends RecursiveAstVisitor { isFieldFormalParameter = parameter.toSource().contains('this.'); isSuperFormalParameter = parameter.toSource().contains('super.'); } + // if (name != null && !isPresentInAll && diff --git a/lib/src/lints/function_lines_of_code/function_lines_of_code_metric.dart b/lib/src/lints/function_lines_of_code/function_lines_of_code_metric.dart index bdbaac40..74387831 100644 --- a/lib/src/lints/function_lines_of_code/function_lines_of_code_metric.dart +++ b/lib/src/lints/function_lines_of_code/function_lines_of_code_metric.dart @@ -16,6 +16,8 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// rules: /// - function_lines_of_code: /// max_lines: 100 +/// excludeNames: +/// - "Build" /// ``` class FunctionLinesOfCodeMetric extends SolidLintRule { @@ -49,6 +51,7 @@ class FunctionLinesOfCodeMetric void checkNode(AstNode node) => _checkNode(resolver, reporter, node); context.registry.addMethodDeclaration(checkNode); + context.registry.addFunctionDeclaration(checkNode); context.registry.addFunctionExpression(checkNode); } @@ -57,6 +60,12 @@ class FunctionLinesOfCodeMetric ErrorReporter reporter, AstNode node, ) { + final functionName = _getFunctionName(node); + if (functionName != null && + config.parameters.excludeNames.contains(functionName)) { + return; + } + final visitor = FunctionLinesOfCodeVisitor(resolver.lineInfo); node.visitChildren(visitor); @@ -75,4 +84,16 @@ class FunctionLinesOfCodeMetric ); } } + + String? _getFunctionName(AstNode node) { + if (node is FunctionDeclaration) { + return node.name.lexeme; + } else if (node is MethodDeclaration) { + return node.name.lexeme; + } else if (node is FunctionExpression) { + return node.declaredElement?.name; + } else { + return null; + } + } } diff --git a/lib/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart b/lib/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart index a04febc9..757f5baf 100644 --- a/lib/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart +++ b/lib/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart @@ -5,16 +5,22 @@ class FunctionLinesOfCodeParameters { /// exceeding this limit triggers a warning. final int maxLines; + /// Function names to be excluded from the rule check + final List excludeNames; + static const _defaultMaxLines = 200; /// Constructor for [FunctionLinesOfCodeParameters] model const FunctionLinesOfCodeParameters({ required this.maxLines, + required this.excludeNames, }); /// Method for creating from json data factory FunctionLinesOfCodeParameters.fromJson(Map json) => FunctionLinesOfCodeParameters( maxLines: json['max_lines'] as int? ?? _defaultMaxLines, + excludeNames: + List.from(json['excludeNames'] as Iterable? ?? []), ); } diff --git a/lib/src/lints/no_magic_number/no_magic_number_rule.dart b/lib/src/lints/no_magic_number/no_magic_number_rule.dart index 1711c17f..9c1a83c6 100644 --- a/lib/src/lints/no_magic_number/no_magic_number_rule.dart +++ b/lib/src/lints/no_magic_number/no_magic_number_rule.dart @@ -59,6 +59,14 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// bool canDrive(int age, {bool isUSA = false}) { /// return isUSA ? age >= 16 : age > 18; // LINT /// } +/// +/// class Circle { +/// final int r; +/// const Circle({required this.r}); +/// } +/// Circle(r: 5); // LINT +/// var circle = Circle(r: 10); // LINT +/// final circle2 = Circle(r: 10); // LINT /// ``` /// /// #### GOOD: @@ -74,6 +82,13 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// bool canDrive(int age, {bool isUSA = false}) { /// return isUSA ? age >= usaDrivingAge : age > worldWideDrivingAge; /// } +/// +/// class Circle { +/// final int r; +/// const Circle({required this.r}); +/// } +/// const Circle(r: 5); +/// const circle = Circle(r: 10) /// ``` /// /// ### Allowed @@ -174,11 +189,24 @@ class NoMagicNumberRule extends SolidLintRule { (l is IntegerLiteral && !config.parameters.allowedNumbers.contains(l.value)); - bool _isNotInsideVariable(Literal l) => - l.thisOrAncestorMatching( - (ancestor) => ancestor is VariableDeclaration, - ) == - null; + bool _isNotInsideVariable(Literal l) { + // Whether we encountered such node, + // This is tracked because [InstanceCreationExpression] can be + // inside [VariableDeclaration] removing unwanted literals + + bool isInstanceCreationExpression = false; + return l.thisOrAncestorMatching((ancestor) { + if (ancestor is InstanceCreationExpression) { + isInstanceCreationExpression = true; + } + if (isInstanceCreationExpression) { + return false; + } else { + return ancestor is VariableDeclaration; + } + }) == + null; + } bool _isNotInDateTime(Literal l) => l.thisOrAncestorMatching( @@ -206,10 +234,9 @@ class NoMagicNumberRule extends SolidLintRule { } bool _isNotInsideConstConstructor(Literal l) => - l.thisOrAncestorMatching( - (ancestor) => - ancestor is InstanceCreationExpression && ancestor.isConst, - ) == + l.thisOrAncestorMatching((ancestor) { + return ancestor is InstanceCreationExpression && ancestor.isConst; + }) == null; bool _isNotInsideIndexExpression(Literal l) => l.parent is! IndexExpression; diff --git a/lib/src/utils/typecast_utils.dart b/lib/src/utils/typecast_utils.dart index be228d16..cb0bfb12 100644 --- a/lib/src/utils/typecast_utils.dart +++ b/lib/src/utils/typecast_utils.dart @@ -28,26 +28,30 @@ class TypeCast { this.isReversed = false, }); - /// Returns the first type from source's supertypes - /// which is corresponding to target or null - DartType? castTypeInHierarchy() { + /// Checks that type checking is unnecessary + bool get isUnnecessaryTypeCheck { + final source = this.source; + + if (_isNullableCompatibility) { + return false; + } + if (source.element == target.element) { - return source; + return _areGenericsWithSameTypeArgs; } - final objectType = source; - if (objectType is InterfaceType) { - return objectType.allSupertypes.firstWhereOrNull( - (value) => value.element == target.element, + if (source is InterfaceType) { + return source.allSupertypes.any( + (e) => e.element == target.element, ); } - return null; + return false; } /// Checks for nullable type casts /// Only one case `Type? is Type` always valid assertion case. - bool get isNullableCompatibility { + bool get _isNullableCompatibility { final isObjectTypeNullable = source.nullabilitySuffix != NullabilitySuffix.none; final isCastedTypeNullable = @@ -56,33 +60,8 @@ class TypeCast { return isObjectTypeNullable && !isCastedTypeNullable; } - /// Checks that type checking is unnecessary - /// [source] is the source expression type - /// [target] is the type against which the expression type is compared - /// and false for positive comparison, i.e. 'is', 'as' or 'whereType' - bool get isUnnecessaryTypeCheck { - if (isNullableCompatibility) { - return false; - } - - final objectCastedType = castTypeInHierarchy(); - if (objectCastedType == null) { - return isReversed; - } - - final objectTypeCast = TypeCast( - source: objectCastedType, - target: target, - ); - if (!objectTypeCast.areGenericsWithSameTypeArgs) { - return false; - } - - return !isReversed; - } - /// Checks for type arguments and compares them - bool get areGenericsWithSameTypeArgs { + bool get _areGenericsWithSameTypeArgs { if (source is DynamicType || target is DynamicType) { return false; } diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index c644d4d1..ea43510f 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -27,6 +27,7 @@ custom_lint: - newline_before_return - no_empty_block - no_equal_then_else + - avoid_debug_print - member_ordering: alphabetize: true order: diff --git a/lint_test/avoid_debug_print_test.dart/avoid_debug_print_prefix_test.dart b/lint_test/avoid_debug_print_test.dart/avoid_debug_print_prefix_test.dart new file mode 100644 index 00000000..1767f8f0 --- /dev/null +++ b/lint_test/avoid_debug_print_test.dart/avoid_debug_print_prefix_test.dart @@ -0,0 +1,25 @@ +// ignore_for_file: unused_local_variable + +import 'package:flutter/foundation.dart' as f; + +/// Test the avoid_debug_print +void avoidDebugPrintTest() { + // expect_lint: avoid_debug_print + f.debugPrint(''); + + // expect_lint: avoid_debug_print + final test = f.debugPrint; + + test('test'); + + // expect_lint: avoid_debug_print + final test2 = f.debugPrint(''); + + debugPrint(); + + debugPrint; +} + +void debugPrint() { + return; +} diff --git a/lint_test/avoid_debug_print_test.dart/avoid_debug_print_test.dart b/lint_test/avoid_debug_print_test.dart/avoid_debug_print_test.dart new file mode 100644 index 00000000..32cdc597 --- /dev/null +++ b/lint_test/avoid_debug_print_test.dart/avoid_debug_print_test.dart @@ -0,0 +1,28 @@ +// ignore_for_file: unused_local_variable + +import 'package:flutter/foundation.dart'; + +/// Test the avoid_debug_print +void avoidDebugPrintTest() { + // expect_lint: avoid_debug_print + debugPrint(''); + + // expect_lint: avoid_debug_print + final test = debugPrint; + + var test2; + + // expect_lint: avoid_debug_print + test2 = debugPrint; + + test.call('test'); + + // expect_lint: avoid_debug_print + final test3 = debugPrint(''); + + someOtherFunction(); +} + +void someOtherFunction() { + print('iii'); +} diff --git a/lint_test/avoid_returning_widget_test.dart b/lint_test/avoid_returning_widget_test.dart index ed366e1a..cc7a5fa1 100644 --- a/lint_test/avoid_returning_widget_test.dart +++ b/lint_test/avoid_returning_widget_test.dart @@ -9,7 +9,25 @@ import 'package:flutter/material.dart'; // expect_lint: avoid_returning_widgets Widget avoidReturningWidgets() => const SizedBox(); -class MyWidget extends StatelessWidget { +class BaseWidget extends StatelessWidget { + const BaseWidget({super.key}); + + // Not allowed even though overriding it is alllowed + // expect_lint: avoid_returning_widgets + Widget get box => SizedBox(); + + // expect_lint: avoid_returning_widgets + Widget decoratedBox() { + return DecoratedBox(decoration: BoxDecoration()); + } + + @override + Widget build(BuildContext context) { + return const Placeholder(); + } +} + +class MyWidget extends BaseWidget { const MyWidget({super.key}); // expect_lint: avoid_returning_widgets @@ -25,8 +43,24 @@ class MyWidget extends StatelessWidget { // expect_lint: avoid_returning_widgets Widget get _test3 => const SizedBox(); + // Allowed + @override + Widget decoratedBox() { + return super.decoratedBox(); + } + + // Allowed + @override + Widget get box => ColoredBox(color: Colors.pink); + + // Allowed @override Widget build(BuildContext context) { return const SizedBox(); } } + +// expect_lint: avoid_returning_widgets +Widget build() { + return Offstage(); +} diff --git a/lint_test/avoid_unnecessary_type_assertions_test.dart b/lint_test/avoid_unnecessary_type_assertions_test.dart index c95e47f6..b3f2f3c6 100644 --- a/lint_test/avoid_unnecessary_type_assertions_test.dart +++ b/lint_test/avoid_unnecessary_type_assertions_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: prefer_const_declarations +// ignore_for_file: prefer_const_declarations, prefer_match_file_name, cyclomatic_complexity, unused_element // ignore_for_file: unnecessary_nullable_for_final_variable_declarations // ignore_for_file: unnecessary_type_check // ignore_for_file: unused_local_variable @@ -34,3 +34,14 @@ void fun() { // casting `Type? is Type` is allowed final castedD = nullableD is double; } + +class _A {} + +class _B extends _A {} + +class _C extends _A {} + +void noLint() { + final _A a = _B(); + if (a is! _C) return; +} diff --git a/lint_test/avoid_unrelated_type_assertions_test.dart b/lint_test/avoid_unrelated_type_assertions_test.dart index eee0a09a..209a5615 100644 --- a/lint_test/avoid_unrelated_type_assertions_test.dart +++ b/lint_test/avoid_unrelated_type_assertions_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: prefer_const_declarations, prefer_match_file_name +// ignore_for_file: prefer_const_declarations, prefer_match_file_name, unused_element // ignore_for_file: unnecessary_nullable_for_final_variable_declarations // ignore_for_file: unused_local_variable @@ -31,3 +31,19 @@ void fun() { // expect_lint: avoid_unrelated_type_assertions final result5 = testMap['A'] is double; } + +class _A {} + +class _B extends _A {} + +class _C {} + +void lint() { + final _A a = _B(); + // Always false + // expect_lint: avoid_unrelated_type_assertions + if (a is _C) return; + // Always true + // expect_lint: avoid_unrelated_type_assertions + if (a is! _C) return; +} diff --git a/lint_test/avoid_unused_parameters_test.dart b/lint_test/avoid_unused_parameters_test.dart index 4d3950f1..b75e120f 100644 --- a/lint_test/avoid_unused_parameters_test.dart +++ b/lint_test/avoid_unused_parameters_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: prefer_const_declarations, prefer_match_file_name +// ignore_for_file: prefer_const_declarations, prefer_match_file_name, avoid_global_state // ignore_for_file: unnecessary_nullable_for_final_variable_declarations // ignore_for_file: unused_local_variable // ignore_for_file: unused_element @@ -9,12 +9,79 @@ import 'package:flutter/material.dart'; /// Check the `avoid_unused_parameters` rule +/// +void testNamed() { + SomeAnotherClass( + func: ({required text}) {}, + ); +} + +typedef MaxFun = int Function(int a, int b); + +typedef Named = String Function({required String text}); + +typedef ReqNamed = void Function({required String text}); + +// expect_lint: avoid_unused_parameters +final MaxFun bad = (int a, int b) => 1; + +final MaxFun good = (int a, _) => a; + +final MaxFun ok = (int _, int __) => 1; + +final MaxFun goodMax = (int a, int b) { + return a + b; +}; + +final Named _named = ({required text}) { + return ''; +}; + +void ch({String text = ''}) {} + +final Named _named2 = ({required text}) { + return text; +}; + +final Named _named3 = ({required text}) => ''; + +final Named _named4 = ({required text}) => text; + +// expect_lint: avoid_unused_parameters +final optional = (int a, [int b = 0]) { + return a; +}; + +// expect_lint: avoid_unused_parameters +final named = (int a, {required int b, int c = 0}) { + return c; +}; + +// good +var k = (String g) { + return g; +}; + +// expect_lint: avoid_unused_parameters +var c = (String g) { + return '0'; +}; + +// expect_lint: avoid_unused_parameters +final MaxFun tetsFun = (int a, int b) { + return 4; +}; // expect_lint: avoid_unused_parameters void fun(String s) { return; } +// expect_lint: avoid_unused_parameters +void fun2(String s) { + return; +} + class TestClass { // expect_lint: avoid_unused_parameters static void staticMethod(int a) {} @@ -34,6 +101,14 @@ class TestClass2 { } class SomeOtherClass { + // expect_lint: avoid_unused_parameters + final MaxFun maxFunLint = (int a, int b) => 1; + + // Good + final MaxFun good = (int a, int b) { + return a * b; + }; + // expect_lint: avoid_unused_parameters void method(String s) { return; @@ -41,6 +116,12 @@ class SomeOtherClass { } class SomeAnotherClass extends SomeOtherClass { + final ReqNamed func; + + SomeAnotherClass({ + required this.func, + }); + @override void method(String s) {} } @@ -67,11 +148,11 @@ void closure(int a) { } } -typedef MaxFun = int Function(int a, int b); - -// Allowed same way as override +// expect_lint: avoid_unused_parameters final MaxFun maxFunInstance = (int a, int b) => 1; +final MaxFun m = (_, __) => 1; + class Foo { final int a; final int? b; @@ -102,8 +183,15 @@ class TestWidget extends StatelessWidget { super.key, // expect_lint: avoid_unused_parameters int a = 1, + // expect_lint: avoid_unused_parameters + String k = '', }); + // expect_lint: avoid_unused_parameters + factory TestWidget.a([int b = 0]) { + return TestWidget(k: ''); + } + @override Widget build(BuildContext context) { return const Placeholder(); diff --git a/lint_test/function_lines_of_code_test/analysis_options.yaml b/lint_test/function_lines_of_code_test/analysis_options.yaml new file mode 100644 index 00000000..51f88abf --- /dev/null +++ b/lint_test/function_lines_of_code_test/analysis_options.yaml @@ -0,0 +1,11 @@ +analyzer: + plugins: + - ../custom_lint + +custom_lint: + rules: + - function_lines_of_code: + max_lines: 5 + excludeNames: + - "longFunctionExcluded" + - "longMethodExcluded" diff --git a/lint_test/function_lines_of_code_test/function_lines_of_code_test.dart b/lint_test/function_lines_of_code_test/function_lines_of_code_test.dart new file mode 100644 index 00000000..aa4211a9 --- /dev/null +++ b/lint_test/function_lines_of_code_test/function_lines_of_code_test.dart @@ -0,0 +1,57 @@ +class ClassWithLongMethods { + int notLongMethod() { + var i = 0; + i++; + i++; + i++; + return i; + } + + // expect_lint: function_lines_of_code + int longMethod() { + var i = 0; + i++; + i++; + i++; + i++; + return i; + } + + // Excluded by excludeNames + int longMethodExcluded() { + var i = 0; + i++; + i++; + i++; + i++; + return i; + } +} + +int notLongFunction() { + var i = 0; + i++; + i++; + i++; + return i; +} + +// expect_lint: function_lines_of_code +int longFunction() { + var i = 0; + i++; + i++; + i++; + i++; + return i; +} + +// Excluded by excludeNames +int longFunctionExcluded() { + var i = 0; + i++; + i++; + i++; + i++; + return i; +} diff --git a/lint_test/lines_of_code_test.dart b/lint_test/lines_of_code_test.dart deleted file mode 100644 index 8afc9c61..00000000 --- a/lint_test/lines_of_code_test.dart +++ /dev/null @@ -1,191 +0,0 @@ -// ignore_for_file: no_magic_number, prefer_match_file_name - -import 'package:test/test.dart'; - -/// Check number of lines fail -/// -/// `function_lines_of_code: max_lines` -/// expect_lint: function_lines_of_code -void linesOfCode() { - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); -} - -class A { - /// expect_lint: function_lines_of_code - void b() { - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - } -} - -class B { - /// expect_lint: function_lines_of_code - void anon() { - /// expect_lint: function_lines_of_code - test("addition", () { - expect(1 + 1, equals(2)); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - test("addition", () { - expect(1 + 1, equals(2)); - }); - }); - } - - void m() { - test("addition", () { - expect(1 + 1, equals(2)); - }); - } -} diff --git a/lint_test/no_magic_number_test.dart b/lint_test/no_magic_number_test.dart index 3f94784e..4bdc69e2 100644 --- a/lint_test/no_magic_number_test.dart +++ b/lint_test/no_magic_number_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: unused_local_variable +// ignore_for_file: unused_local_variable, avoid_returning_widgets // ignore_for_file: prefer_match_file_name // ignore_for_file: avoid_unused_parameters // ignore_for_file: no_empty_block @@ -15,7 +15,7 @@ double correctCircumference(double radius) => radiusToDiameterCoefficient * pi * radius; bool canDrive(int age, {bool isUSA = false}) { -// expect_lint: no_magic_number + // expect_lint: no_magic_number return isUSA ? age >= 16 : age > 18; } @@ -111,7 +111,39 @@ Widget build() { return MyWidget( // expect_lint: no_magic_number decoration: MyWidgetDecoration(size: 12), + // expect_lint: no_magic_number value: 23, ); } + +class TestOperation { + final double res; + + const TestOperation({required this.res}); +} + +const n = 15; +const m = 20; + +void checkOperationInConstructor() { + // expect_lint: no_magic_number + TestOperation(res: (5 / 5) * 20); + + // expect_lint: no_magic_number + var variable = TestOperation(res: (n / m) * 20); + + // expect_lint: no_magic_number + final finalVar = TestOperation(res: (10 / m + 4 + n)); + + // Allowed for constant expressions + const constVar = TestOperation(res: (10 / m + 4 + n)); + + var constVar2 = const TestOperation(res: 15 + (10 / n)); + + final l = [ + // expect_lint: no_magic_number + TestOperation(res: 8), + const TestOperation(res: 9), + ]; +} diff --git a/pubspec.yaml b/pubspec.yaml index f70238d2..6ae42e53 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -2,7 +2,7 @@ name: solid_lints description: Lints for Dart and Flutter based on software industry standards and best practices. -version: 0.1.4 +version: 0.1.5 homepage: https://github.com/solid-software/solid_lints/ documentation: https://solid-software.github.io/solid_lints/docs/intro topics: [lints, linter, lint, analysis, analyzer]