diff --git a/lib/main.dart b/lib/main.dart new file mode 100644 index 00000000..6e7694bb --- /dev/null +++ b/lib/main.dart @@ -0,0 +1,75 @@ +import 'package:analysis_server_plugin/plugin.dart'; +import 'package:analysis_server_plugin/registry.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; +import 'package:solid_lints/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/fixes/avoid_final_with_getter_fix.dart'; +import 'package:solid_lints/src/lints/avoid_global_state/avoid_global_state_rule.dart'; +import 'package:solid_lints/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart'; +import 'package:solid_lints/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart'; +import 'package:solid_lints/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/fixes/avoid_unnecessary_type_assertions_fix.dart'; +import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart'; +import 'package:solid_lints/src/lints/double_literal_format/fixes/double_literal_format_fix.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart'; +import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; + +/// The entry point for the Solid Lints analyser server plugin. +/// +/// This plugin integrates custom lint rules into the Dart analysis server, +/// allowing them to run during static analysis. +final plugin = SolidLintsPlugin(); + +/// An analysis server plugin that provides Solid lint rules. +/// +/// This plugin registers custom lint rules and enables them to be executed +/// by the Dart analyzer during code analysis. +class SolidLintsPlugin extends Plugin { + @override + String get name => 'solid_lints'; + + @override + void register(PluginRegistry registry) { + final analysisLoader = AnalysisOptionsLoader(); + + final avoidUnnecessaryTypeAssertionsRule = + AvoidUnnecessaryTypeAssertionsRule(); + final doubleLiteralFormatRule = DoubleLiteralFormatRule(); + final lintRules = [ + AvoidFinalWithGetterRule(), + AvoidGlobalStateRule(), + AvoidNonNullAssertionRule(), + avoidUnnecessaryTypeAssertionsRule, + AvoidDebugPrintInReleaseRule(), + doubleLiteralFormatRule, + ProperSuperCallsRule(), + AvoidReturningWidgetsRule( + analysisOptionsLoader: analysisLoader, + parametersParser: AvoidReturningWidgetsParameters.fromJson, + ), + FunctionLinesOfCodeRule( + analysisOptionsLoader: analysisLoader, + parametersParser: FunctionLinesOfCodeParameters.fromJson, + ), + ]; + + for (final lintRule in lintRules) { + registry.registerLintRule(lintRule); + } + + for (final code in doubleLiteralFormatRule.diagnosticCodes) { + registry.registerFixForRule(code, DoubleLiteralFormatFix.new); + } + + registry.registerFixForRule( + AvoidFinalWithGetterRule.code, + AvoidFinalWithGetterFix.new, + ); + registry.registerFixForRule( + avoidUnnecessaryTypeAssertionsRule.diagnosticCode, + AvoidUnnecessaryTypeAssertionsFix.new, + ); + } +} diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart deleted file mode 100644 index 4fc66f58..00000000 --- a/lib/solid_lints.dart +++ /dev/null @@ -1,77 +0,0 @@ -library solid_metrics; - -import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:solid_lints/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart'; -import 'package:solid_lints/src/lints/avoid_final_with_getter/avoid_final_with_getter_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'; -import 'package:solid_lints/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart'; -import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart'; -import 'package:solid_lints/src/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart'; -import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart'; -import 'package:solid_lints/src/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_rule.dart'; -import 'package:solid_lints/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule.dart'; -import 'package:solid_lints/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart'; -import 'package:solid_lints/src/lints/avoid_using_api/avoid_using_api_rule.dart'; -import 'package:solid_lints/src/lints/cyclomatic_complexity/cyclomatic_complexity_rule.dart'; -import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart'; -import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; -import 'package:solid_lints/src/lints/member_ordering/member_ordering_rule.dart'; -import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart'; -import 'package:solid_lints/src/lints/newline_before_return/newline_before_return_rule.dart'; -import 'package:solid_lints/src/lints/no_empty_block/no_empty_block_rule.dart'; -import 'package:solid_lints/src/lints/no_equal_then_else/no_equal_then_else_rule.dart'; -import 'package:solid_lints/src/lints/no_magic_number/no_magic_number_rule.dart'; -import 'package:solid_lints/src/lints/number_of_parameters/number_of_parameters_rule.dart'; -import 'package:solid_lints/src/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart'; -import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_rule.dart'; -import 'package:solid_lints/src/lints/prefer_first/prefer_first_rule.dart'; -import 'package:solid_lints/src/lints/prefer_last/prefer_last_rule.dart'; -import 'package:solid_lints/src/lints/prefer_match_file_name/prefer_match_file_name_rule.dart'; -import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; - -/// Creates a plugin for our custom linter -PluginBase createPlugin() => _SolidLints(); - -/// Initialize custom solid lints -class _SolidLints extends PluginBase { - @override - List getLintRules(CustomLintConfigs configs) { - final List supportedRules = [ - CyclomaticComplexityRule.createRule(configs), - NumberOfParametersRule.createRule(configs), - FunctionLinesOfCodeRule.createRule(configs), - AvoidNonNullAssertionRule.createRule(configs), - AvoidLateKeywordRule.createRule(configs), - AvoidGlobalStateRule.createRule(configs), - AvoidReturningWidgetsRule.createRule(configs), - DoubleLiteralFormatRule.createRule(configs), - AvoidUnnecessaryTypeAssertions.createRule(configs), - AvoidUnnecessarySetStateRule.createRule(configs), - AvoidUnnecessaryTypeCastsRule.createRule(configs), - AvoidUnrelatedTypeAssertionsRule.createRule(configs), - AvoidUnusedParametersRule.createRule(configs), - AvoidUsingApiRule.createRule(configs), - NewlineBeforeReturnRule.createRule(configs), - NoEmptyBlockRule.createRule(configs), - NoEqualThenElseRule.createRule(configs), - MemberOrderingRule.createRule(configs), - NoMagicNumberRule.createRule(configs), - PreferConditionalExpressionsRule.createRule(configs), - PreferFirstRule.createRule(configs), - PreferLastRule.createRule(configs), - PreferMatchFileNameRule.createRule(configs), - ProperSuperCallsRule.createRule(configs), - AvoidDebugPrintInReleaseRule.createRule(configs), - PreferEarlyReturnRule.createRule(configs), - AvoidFinalWithGetterRule.createRule(configs), - NamedParametersOrderingRule.createRule(configs), - AvoidUnnecessaryReturnVariableRule.createRule(configs), - ]; - - // Return only enabled rules - return supportedRules.where((r) => r.enabled).toList(); - } -} diff --git a/lib/src/common/parameter_parser/analysis_options_loader.dart b/lib/src/common/parameter_parser/analysis_options_loader.dart new file mode 100644 index 00000000..ab22ecc9 --- /dev/null +++ b/lib/src/common/parameter_parser/analysis_options_loader.dart @@ -0,0 +1,109 @@ +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/file_system/file_system.dart'; +import 'package:analyzer/file_system/physical_file_system.dart'; +import 'package:solid_lints/src/common/parameter_parser/cached_package_rules.dart'; +import 'package:yaml/yaml.dart'; + +/// Loads and parses analysis options from a Dart project's YAML file. +class AnalysisOptionsLoader { + final ResourceProvider _resourceProvider; + final Map _rulesCache = {}; + + /// Creates an instance of [AnalysisOptionsLoader] + AnalysisOptionsLoader({ResourceProvider? resourceProvider}) + : _resourceProvider = + resourceProvider ?? PhysicalResourceProvider.INSTANCE; + + /// Gets the options for a specific rule by its name. + Map? getRuleOptions(RuleContext context, String ruleName) => + _withNearestAnalysisOptionsFilePathForContext?>( + context, + (path) => _rulesCache[path]?.rules[ruleName], + ); + + /// Loads lint rules from the analysis options file for all rules + /// using the provided [RuleContext]. + void loadRulesOptionsFromContext(RuleContext context) => + _withNearestAnalysisOptionsFilePathForContext( + context, + _loadRulesOptionsIfNewer, + ); + + T? _withNearestAnalysisOptionsFilePathForContext( + RuleContext context, + T Function(String) f, + ) { + final packageRootPath = context.package?.root.path; + if (packageRootPath == null) return null; + + final yamlPath = _findNearestAnalysisOptionsFilePath(packageRootPath); + if (yamlPath == null) return null; + + return f(yamlPath); + } + + void _loadRulesOptionsIfNewer(String yamlPath) { + final analysisOptionsFile = _resourceProvider.getFile(yamlPath); + final modificationStamp = analysisOptionsFile.modificationStamp; + final cachedRules = _rulesCache[yamlPath]; + + if (cachedRules?.modificationStamp == modificationStamp) { + return; + } + + final rules = _getRules(analysisOptionsFile); + _rulesCache[yamlPath] = CachedPackageRules( + modificationStamp: modificationStamp, + rules: rules, + ); + } + + String? _findNearestAnalysisOptionsFilePath(String packageRootPath) { + final pathContext = _resourceProvider.pathContext; + String currentDirectoryPath = packageRootPath; + + while (pathContext.dirname(currentDirectoryPath) != currentDirectoryPath) { + final candidatePath = + pathContext.join(currentDirectoryPath, 'analysis_options.yaml'); + final candidateFile = _resourceProvider.getFile(candidatePath); + + if (candidateFile.exists) { + return candidatePath; + } + + final parentDir = pathContext.dirname(currentDirectoryPath); + currentDirectoryPath = parentDir; + } + + return null; + } + + Map> _getRules(File? analysisOptionsFile) { + if (analysisOptionsFile == null || !analysisOptionsFile.exists) { + return {}; + } + + final optionsString = analysisOptionsFile.readAsStringSync(); + Object? yaml; + try { + yaml = loadYaml(optionsString) as Object?; + } catch (err) { + return {}; + } + + if (yaml + case {'plugins': {'solid_lints': {'diagnostics': final diagnostics?}}} + when diagnostics is Map) { + return Map.fromEntries( + diagnostics.entries.where((e) => e.key is String && e.value is Map).map( + (e) => MapEntry( + e.key as String, + Map.from(e.value as Map), + ), + ), + ); + } + + return {}; + } +} diff --git a/lib/src/common/parameter_parser/cached_package_rules.dart b/lib/src/common/parameter_parser/cached_package_rules.dart new file mode 100644 index 00000000..a65201d9 --- /dev/null +++ b/lib/src/common/parameter_parser/cached_package_rules.dart @@ -0,0 +1,14 @@ +/// Cached rules for a dart package +class CachedPackageRules { + /// The last modification stamp of the analysis options file + final int modificationStamp; + + /// Cached rules options by rule name for the package + final Map> rules; + + /// Creates an instance of [CachedPackageRules] + const CachedPackageRules({ + required this.modificationStamp, + required this.rules, + }); +} diff --git a/lib/src/common/parameters/excluded_identifiers_list_parameter.dart b/lib/src/common/parameters/excluded_identifiers_list_parameter.dart index 8a4b1097..526201a4 100644 --- a/lib/src/common/parameters/excluded_identifiers_list_parameter.dart +++ b/lib/src/common/parameters/excluded_identifiers_list_parameter.dart @@ -82,7 +82,7 @@ class ExcludedIdentifiersListParameter { final classDeclaration = node.thisOrAncestorOfType(); return classDeclaration != null && - classDeclaration.name.toString() == className; + classDeclaration.namePart.typeName.lexeme == className; } } } diff --git a/lib/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart b/lib/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart index e9b52edb..a5d3d44b 100644 --- a/lib/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart +++ b/lib/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart @@ -1,10 +1,8 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/ast/syntactic_entity.dart'; -import 'package:analyzer/dart/ast/token.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'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:solid_lints/src/lints/avoid_debug_print_in_release/visitors/avoid_debug_print_in_release_visitor.dart'; /// An `avoid_debug_print_in_release` rule which forbids calling or referencing /// debugPrint function from flutter/foundation in release mode. @@ -31,176 +29,32 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ``` /// /// -class AvoidDebugPrintInReleaseRule extends SolidLintRule { - /// This lint rule represents - /// the error when debugPrint is called - static const lintName = 'avoid_debug_print_in_release'; - static const String _kReleaseModePath = - 'package:flutter/src/foundation/constants.dart'; - static const String _kReleaseModeName = 'kReleaseMode'; - static const _debugPrintPath = 'package:flutter/src/foundation/print.dart'; - static const _debugPrintName = 'debugPrint'; +class AvoidDebugPrintInReleaseRule extends AnalysisRule { + /// The name of the lint + static const String lintName = 'avoid_debug_print_in_release'; - AvoidDebugPrintInReleaseRule._(super.config); + /// Lint code used for suppression and reporting. + static const LintCode _code = LintCode( + lintName, + 'Avoid debugPrint in release mode.', + correctionMessage: 'Wrap in a kReleaseMode check or use a logging package.', + ); - /// Creates a new instance of [AvoidDebugPrintInReleaseRule] - /// based on the lint configuration. - factory AvoidDebugPrintInReleaseRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => """ -Avoid using 'debugPrint' in release mode. Wrap -your `debugPrint` call in a `!kReleaseMode` check.""", - ); + /// Creates an instance of [AvoidDebugPrintInReleaseRule]. + AvoidDebugPrintInReleaseRule() + : super(name: lintName, description: 'Avoid debugPrint in release mode.'); - return AvoidDebugPrintInReleaseRule._(rule); - } + @override + LintCode get diagnosticCode => _code; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext 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 DiagnosticReporter reporter, - }) { - if (!_isDebugPrintNode(identifier)) { - return; - } - - final debugCheck = node.thisOrAncestorMatching( - (node) { - if (node is IfStatement) { - return _isNotReleaseCheck(node.expression); - } - - return false; - }, - ); - - if (debugCheck != null) { - return; - } - - reporter.atNode(node, code); - } - - /// Returns null if doesn't 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 DiagnosticReporter reporter, - }) { - final rightOperand = _getRightOperand(node.childEntities.toList()); - - if (rightOperand == null || rightOperand is! Identifier) { - return; - } - - _checkIdentifier( - identifier: rightOperand, - node: node, - reporter: reporter, - ); - } - - bool _isDebugPrintNode(Identifier node) { - final String name; - final String sourcePath; - switch (node) { - case PrefixedIdentifier(): - final prefix = node.prefix.name; - name = node.name.replaceAll('$prefix.', ''); - sourcePath = node.element?.library?.uri.toString() ?? ''; - case SimpleIdentifier(): - name = node.name; - sourcePath = node.element?.library?.uri.toString() ?? ''; - - default: - return false; - } - - return name == _debugPrintName && sourcePath == _debugPrintPath; - } - - bool _isNotReleaseCheck(Expression node) { - if (node.childEntities.toList() - case [ - final Token token, - final Identifier identifier, - ]) { - return token.type == TokenType.BANG && - _isReleaseModeIdentifier(identifier); - } - - return false; - } - - bool _isReleaseModeIdentifier(Identifier node) { - final String name; - final String sourcePath; - - switch (node) { - case PrefixedIdentifier(): - final prefix = node.prefix.name; - - name = node.name.replaceAll('$prefix.', ''); - sourcePath = node.element?.library?.uri.toString() ?? ''; - case SimpleIdentifier(): - name = node.name; - sourcePath = node.element?.library?.uri.toString() ?? ''; - default: - return false; - } - - return name == _kReleaseModeName && sourcePath == _kReleaseModePath; + final visitor = AvoidDebugPrintInReleaseVisitor(this); + registry.addMethodInvocation(this, visitor); + registry.addSimpleIdentifier(this, visitor); } } diff --git a/lib/src/lints/avoid_debug_print_in_release/visitors/avoid_debug_print_in_release_visitor.dart b/lib/src/lints/avoid_debug_print_in_release/visitors/avoid_debug_print_in_release_visitor.dart new file mode 100644 index 00000000..8b794a1b --- /dev/null +++ b/lib/src/lints/avoid_debug_print_in_release/visitors/avoid_debug_print_in_release_visitor.dart @@ -0,0 +1,86 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart'; + +/// Visitor for [AvoidDebugPrintInReleaseRule]. +class AvoidDebugPrintInReleaseVisitor extends SimpleAstVisitor { + /// The rule associated with this visitor. + final AvoidDebugPrintInReleaseRule rule; + static const _foundationUri = 'package:flutter/foundation.dart'; + static const _debugPrint = 'debugPrint'; + static const _kReleaseMode = 'kReleaseMode'; + static const _kDebugMode = 'kDebugMode'; + static const _callMethod = 'call'; + + /// Creates an instance of [AvoidDebugPrintInReleaseVisitor]. + AvoidDebugPrintInReleaseVisitor(this.rule); + + @override + void visitMethodInvocation(MethodInvocation node) { + final target = node.target; + final methodName = node.methodName; + + if (methodName.name == _callMethod && target is SimpleIdentifier) { + _check(node, target); + return; + } + + _check(node, methodName); + } + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + if (node.parent is! MethodInvocation) { + _check(node, node); + } + } + + void _check(AstNode node, SimpleIdentifier identifier) { + final element = identifier.element; + if (element == null) return; + + if (element.name == _debugPrint) { + final sourceUri = element.library?.uri.toString() ?? ''; + + final isFlutterFoundation = sourceUri.contains( + _foundationUri, + ); + + if (isFlutterFoundation) { + if (!_isWrappedInReleaseCheck(node)) { + rule.reportAtNode(identifier); + } + } + } + } + + bool _isWrappedInReleaseCheck(AstNode node) { + AstNode? parent = node.parent; + + while (parent != null) { + if (parent is IfStatement) { + final condition = parent.expression; + + if (_isNotReleaseModeCheck(condition) || _isDebugModeCheck(condition)) { + return true; + } + } + + parent = parent.parent; + } + + return false; + } + + bool _isNotReleaseModeCheck(Expression condition) { + return condition is PrefixExpression && + condition.operator.type == TokenType.BANG && + condition.operand is SimpleIdentifier && + (condition.operand as SimpleIdentifier).name == _kReleaseMode; + } + + bool _isDebugModeCheck(Expression condition) { + return condition is SimpleIdentifier && condition.name == _kDebugMode; + } +} diff --git a/lib/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart b/lib/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart index d495c5db..727e3dfe 100644 --- a/lib/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart +++ b/lib/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart @@ -1,12 +1,8 @@ -import 'package:analyzer/diagnostic/diagnostic.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:analyzer/source/source_range.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/avoid_final_with_getter/visitors/avoid_final_with_getter_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; - -part 'fixes/avoid_final_with_getter_fix.dart'; /// Avoid using final private fields with getters. /// @@ -34,45 +30,32 @@ part 'fixes/avoid_final_with_getter_fix.dart'; /// } /// ``` /// -class AvoidFinalWithGetterRule extends SolidLintRule { +class AvoidFinalWithGetterRule extends AnalysisRule { /// This lint rule represents /// the error whether we use final private fields with getters. static const lintName = 'avoid_final_with_getter'; - final _diagnosticsInfoExpando = Expando(); - - AvoidFinalWithGetterRule._(super.config); + /// The code to report for a violation + static const LintCode code = LintCode( + lintName, + 'Avoid final private fields with getters.', + correctionMessage: 'Remove the getter and make the field public.', + ); /// Creates a new instance of [AvoidFinalWithGetterRule] - /// based on the lint configuration. - factory AvoidFinalWithGetterRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => 'Avoid final private fields with getters.', - ); + AvoidFinalWithGetterRule() + : super(name: lintName, description: code.problemMessage); - return AvoidFinalWithGetterRule._(rule); - } + @override + LintCode get diagnosticCode => code; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addCompilationUnit((node) { - final visitor = AvoidFinalWithGetterVisitor(); - node.accept(visitor); + final visitor = AvoidFinalWithGetterVisitor(this); - for (final element in visitor.getters) { - final diagnostic = reporter.atNode(element.getter, code); - - _diagnosticsInfoExpando[diagnostic] = element; - } - }); + registry.addCompilationUnit(this, visitor); } - - @override - List getFixes() => [_FinalWithGetterFix(_diagnosticsInfoExpando)]; } diff --git a/lib/src/lints/avoid_final_with_getter/fixes/avoid_final_with_getter_fix.dart b/lib/src/lints/avoid_final_with_getter/fixes/avoid_final_with_getter_fix.dart index 98e08fb4..dd810642 100644 --- a/lib/src/lints/avoid_final_with_getter/fixes/avoid_final_with_getter_fix.dart +++ b/lib/src/lints/avoid_final_with_getter/fixes/avoid_final_with_getter_fix.dart @@ -1,40 +1,84 @@ -part of '../avoid_final_with_getter_rule.dart'; +import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; +import 'package:analysis_server_plugin/edit/dart/dart_fix_kind_priority.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/visitors/variable_references_visitor.dart'; -class _FinalWithGetterFix extends DartFix { - final Expando _diagnosticsInfoExpando; +/// A Quick fix for [AvoidFinalWithGetterRule] rule +class AvoidFinalWithGetterFix extends ResolvedCorrectionProducer { + static const _avoidFinalWithGetterKind = FixKind( + 'solid_lints.fix.${AvoidFinalWithGetterRule.lintName}', + DartFixKindPriority.standard, + "Remove the getter and make the field public", + ); - _FinalWithGetterFix(this._diagnosticsInfoExpando); + /// Creates a new instance of [AvoidFinalWithGetterFix] + AvoidFinalWithGetterFix({required super.context}); @override - void run( - CustomLintResolver resolver, - ChangeReporter reporter, - CustomLintContext context, - Diagnostic diagnostic, - List others, - ) { - context.registry.addMethodDeclaration((node) { - if (!diagnostic.sourceRange.intersects(node.sourceRange)) return; - - final info = _diagnosticsInfoExpando[diagnostic]; - if (info == null) return; - - _addReplacement(reporter, info); - }); - } + CorrectionApplicability get applicability => + CorrectionApplicability.acrossFiles; + + @override + FixKind get fixKind => _avoidFinalWithGetterKind; + + @override + Future compute(ChangeBuilder builder) async { + final getterNode = node; + if (getterNode + case MethodDeclaration( + isGetter: true, + declaredFragment: ExecutableFragment( + element: GetterElement( + isAbstract: false, + isPublic: true, + ), + ), + )) { + final compilationUnit = node.thisOrAncestorOfType(); + if (compilationUnit == null) return; + + final getterVariableVisitor = GetterVariableVisitor(getterNode); + compilationUnit.accept(getterVariableVisitor); + + final variableDeclaration = getterVariableVisitor.variable; + if (variableDeclaration == null) return; + + final referencesVisitor = VariableReferencesVisitor(variableDeclaration); + compilationUnit.accept(referencesVisitor); + + final variableReferences = referencesVisitor.references; + + final variableName = variableDeclaration.name.lexeme; + final newPublicVariableName = variableName.startsWith('_') + ? variableName.substring(1) + : variableName; + + await builder.addDartFileEdit(file, (builder) { + builder.addDeletion(getterNode.sourceRange); + + builder.addSimpleReplacement( + variableDeclaration.name.sourceRange, + newPublicVariableName, + ); + + for (final reference in variableReferences) { + if (reference.sourceRange.intersects(getterNode.sourceRange)) { + continue; + } + + builder.addSimpleReplacement( + reference.sourceRange, + newPublicVariableName, + ); + } - void _addReplacement( - ChangeReporter reporter, - FinalWithGetterInfo info, - ) { - final changeBuilder = reporter.createChangeBuilder( - message: "Remove getter and make variable public.", - priority: 1, - ); - - changeBuilder.addDartFileEdit((builder) { - builder.addDeletion(info.getter.sourceRange); - builder.addDeletion(SourceRange(info.variable.name.offset, 1)); - }); + builder.format(compilationUnit.sourceRange); + }); + } } } diff --git a/lib/src/lints/avoid_final_with_getter/utils/getter_reference_id.dart b/lib/src/lints/avoid_final_with_getter/utils/getter_reference_id.dart new file mode 100644 index 00000000..0aa47984 --- /dev/null +++ b/lib/src/lints/avoid_final_with_getter/utils/getter_reference_id.dart @@ -0,0 +1,37 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/element.dart'; + +/// Extension method to get the reference id of the getter. +extension GetterReferenceId on MethodDeclaration { + /// Get the reference id of the getter. + int? get getterReferenceId { + final returnExpression = switch (body) { + ExpressionFunctionBody(expression: final expr) || + BlockFunctionBody( + block: Block( + statements: [ + ReturnStatement(expression: final expr?), + ], + ) + ) => + expr, + _ => null, + }; + + final identifier = switch (returnExpression) { + SimpleIdentifier() => returnExpression, + PropertyAccess( + target: ThisExpression(), + :final propertyName, + ) => + propertyName, + _ => null, + }; + + return switch (identifier) { + SimpleIdentifier(element: PropertyAccessorElement(:final variable)) => + variable.id, + _ => null, + }; + } +} diff --git a/lib/src/lints/avoid_final_with_getter/visitors/avoid_final_with_getter_visitor.dart b/lib/src/lints/avoid_final_with_getter/visitors/avoid_final_with_getter_visitor.dart index 6e5bfbf1..ee1aa7de 100644 --- a/lib/src/lints/avoid_final_with_getter/visitors/avoid_final_with_getter_visitor.dart +++ b/lib/src/lints/avoid_final_with_getter/visitors/avoid_final_with_getter_visitor.dart @@ -1,18 +1,24 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element.dart'; -import 'package:solid_lints/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/utils/getter_reference_id.dart'; /// A visitor that checks for final private fields with getters. /// If a final private field has a getter, it is considered as a public field. class AvoidFinalWithGetterVisitor extends RecursiveAstVisitor { - final _getters = {}; + final AvoidFinalWithGetterRule _rule; - /// List of getters - Set get getters => _getters; + final _gettersPairLookup = {}; + final _fieldsPairLookup = {}; + + /// Creates a new instance of [AvoidFinalWithGetterVisitor] + AvoidFinalWithGetterVisitor(this._rule); @override void visitMethodDeclaration(MethodDeclaration node) { + super.visitMethodDeclaration(node); + if (node case MethodDeclaration( isGetter: true, @@ -21,29 +27,36 @@ class AvoidFinalWithGetterVisitor extends RecursiveAstVisitor { isAbstract: false, isPublic: true, ) - ) + ), + getterReferenceId: final getterId?, )) { - final visitor = GetterVariableVisitor(node); - node.parent?.accept(visitor); + _gettersPairLookup[getterId] = node; - final variable = visitor.variable; - - if (variable != null) { - _getters.add(FinalWithGetterInfo(node, variable)); + if (_fieldsPairLookup.containsKey(getterId)) { + _rule.reportAtNode(node); } } - super.visitMethodDeclaration(node); } -} -/// Information about the final private field with a getter. -class FinalWithGetterInfo { - /// The getter method declaration. - final MethodDeclaration getter; + @override + void visitVariableDeclaration(VariableDeclaration node) { + super.visitVariableDeclaration(node); - /// The variable declaration. - final VariableDeclaration variable; + if (node + case VariableDeclaration( + declaredFragment: VariableFragment( + element: VariableElement( + isPrivate: true, + isFinal: true, + id: final variableId, + ) + ) + )) { + _fieldsPairLookup[variableId] = node; - /// Creates a new instance of [FinalWithGetterInfo] - const FinalWithGetterInfo(this.getter, this.variable); + if (_gettersPairLookup[variableId] case final getter?) { + _rule.reportAtNode(getter); + } + } + } } diff --git a/lib/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart b/lib/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart index 978b44d6..0e9b4195 100644 --- a/lib/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart +++ b/lib/src/lints/avoid_final_with_getter/visitors/getter_variable_visitor.dart @@ -1,9 +1,9 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/utils/getter_reference_id.dart'; -/// A visitor that checks the association of the getter with -/// the final private variable +/// A visitor that gets the final private variable associated with the getter. class GetterVariableVisitor extends RecursiveAstVisitor { final int? _getterId; VariableDeclaration? _variable; @@ -17,29 +17,19 @@ class GetterVariableVisitor extends RecursiveAstVisitor { @override void visitVariableDeclaration(VariableDeclaration node) { - final element = node.declaredFragment?.element; - if (element != null && - element.isPrivate && - element.isFinal && - element.id == _getterId) { + if (node + case VariableDeclaration( + declaredFragment: VariableFragment( + element: VariableElement( + isPrivate: true, + isFinal: true, + :final id, + ) + ) + ) when id == _getterId) { _variable = node; } super.visitVariableDeclaration(node); } } - -extension on MethodDeclaration { - int? get getterReferenceId { - if (body - case ExpressionFunctionBody( - expression: SimpleIdentifier( - element: PropertyAccessorElement(:final variable) - ) - )) { - return variable.id; - } - - return null; - } -} diff --git a/lib/src/lints/avoid_final_with_getter/visitors/variable_references_visitor.dart b/lib/src/lints/avoid_final_with_getter/visitors/variable_references_visitor.dart new file mode 100644 index 00000000..9f3fee2c --- /dev/null +++ b/lib/src/lints/avoid_final_with_getter/visitors/variable_references_visitor.dart @@ -0,0 +1,32 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/syntactic_entity.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; + +/// Finds all references to a variable declaration. +class VariableReferencesVisitor extends RecursiveAstVisitor { + final VariableDeclaration _variableDeclaration; + final List _references = []; + + /// List of found references to the variable declaration. + List get references => _references; + + /// Creates a new instance of [VariableReferencesVisitor] + VariableReferencesVisitor(this._variableDeclaration); + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + super.visitSimpleIdentifier(node); + + final variableId = _variableDeclaration.declaredFragment?.element.id; + final referencedVariableId = switch (node.element) { + PropertyAccessorElement(:final variable) => variable.id, + final element? => element.id, + _ => null, + }; + + if (referencedVariableId == variableId) { + _references.add(node); + } + } +} diff --git a/lib/src/lints/avoid_global_state/avoid_global_state_rule.dart b/lib/src/lints/avoid_global_state/avoid_global_state_rule.dart index f8211320..6ee78241 100644 --- a/lib/src/lints/avoid_global_state/avoid_global_state_rule.dart +++ b/lib/src/lints/avoid_global_state/avoid_global_state_rule.dart @@ -1,8 +1,8 @@ -import 'package:analyzer/dart/ast/ast.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'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:solid_lints/src/lints/avoid_global_state/visitors/avoid_global_state_visitor.dart'; /// Avoid top-level and static mutable variables. /// @@ -23,7 +23,6 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// } /// ``` /// -/// /// #### GOOD: /// /// ```dart @@ -35,49 +34,36 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// static final int globalFinal = 1; /// } /// ``` -class AvoidGlobalStateRule extends SolidLintRule { - /// This lint rule represents - /// the error whether we use global state. - static const lintName = 'avoid_global_state'; +class AvoidGlobalStateRule extends AnalysisRule { + /// Lint name used for suppression and reporting. + static const String lintName = 'avoid_global_state'; - AvoidGlobalStateRule._(super.config); + /// Lint code used for suppression and reporting. + static const LintCode _code = LintCode( + lintName, + 'Avoid variables that can be globally mutated.', + correctionMessage: + 'Prefer using final/const or a state management solution.', + ); - /// Creates a new instance of [AvoidGlobalStateRule] - /// based on the lint configuration. - factory AvoidGlobalStateRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => 'Avoid variables that can be globally mutated.', - ); + /// Creates an instance of [AvoidGlobalStateRule]. + AvoidGlobalStateRule() + : super( + name: lintName, + description: 'Avoid top-level or static mutable variables ', + ); - return AvoidGlobalStateRule._(rule); - } + @override + LintCode get diagnosticCode => _code; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addTopLevelVariableDeclaration( - (node) => node.variables.variables - .where((variable) => variable.isPublicMutable) - .forEach((node) => reporter.atNode(node, code)), - ); - context.registry.addFieldDeclaration((node) { - if (!node.isStatic) return; - node.fields.variables - .where((variable) => variable.isPublicMutable) - .forEach((node) => reporter.atNode(node, code)); - }); - } -} - -extension on VariableDeclaration { - bool get isMutable => !isFinal && !isConst; + final visitor = AvoidGlobalStateVisitor(this); - bool get isPrivate => declaredFragment?.element.isPrivate ?? false; - - bool get isPublicMutable => isMutable && !isPrivate; + registry.addTopLevelVariableDeclaration(this, visitor); + registry.addFieldDeclaration(this, visitor); + } } diff --git a/lib/src/lints/avoid_global_state/visitors/avoid_global_state_visitor.dart b/lib/src/lints/avoid_global_state/visitors/avoid_global_state_visitor.dart new file mode 100644 index 00000000..1f5a1876 --- /dev/null +++ b/lib/src/lints/avoid_global_state/visitors/avoid_global_state_visitor.dart @@ -0,0 +1,51 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/avoid_global_state/avoid_global_state_rule.dart'; + +/// Visitor for [AvoidGlobalStateRule]. +class AvoidGlobalStateVisitor extends SimpleAstVisitor { + /// The rule this visitor is associated with. + final AvoidGlobalStateRule rule; + + /// Creates an instance of [AvoidGlobalStateVisitor]. + AvoidGlobalStateVisitor(this.rule); + + @override + void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) { + for (final variable in node.variables.variables) { + if (_isPublicMutable(variable)) { + rule.reportAtNode(variable); + } + } + } + + @override + void visitFieldDeclaration(FieldDeclaration node) { + if (!node.isStatic) return; + + for (final variable in node.fields.variables) { + if (_isPublicMutable(variable)) { + rule.reportAtNode(variable); + } + } + } + + /// Returns true if the variable is mutable and not private. + bool _isPublicMutable(VariableDeclaration variable) { + return _isMutable(variable) && !_isPrivate(variable); + } + + /// A variable is mutable if it is not final or const. + bool _isMutable(VariableDeclaration variable) { + final parent = variable.parent; + return parent is VariableDeclarationList && + !parent.isFinal && + !parent.isConst; + } + + /// A variable is private if its element is private. + bool _isPrivate(VariableDeclaration variable) { + final element = variable.declaredFragment?.element; + return element?.isPrivate ?? false; + } +} diff --git a/lib/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart b/lib/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart index 2b27901d..ba98b75a 100644 --- a/lib/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart +++ b/lib/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart @@ -1,10 +1,8 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/ast/token.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'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:solid_lints/src/lints/avoid_non_null_assertion/visitors/avoid_non_null_assertion_visitor.dart'; /// Rule which warns about usages of bang operator ("!") /// as it may result in unexpected runtime exceptions. @@ -37,56 +35,32 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// final map = {'key': 'value'}; /// map['key']!; /// ``` -class AvoidNonNullAssertionRule extends SolidLintRule { - /// This lint rule represents - /// the error whether we use bang operator. - static const lintName = 'avoid_non_null_assertion'; +class AvoidNonNullAssertionRule extends AnalysisRule { + /// Name of the lint + static const String lintName = 'avoid_non_null_assertion'; - AvoidNonNullAssertionRule._(super.config); + /// Lint code used for suppression and reporting + static const LintCode _code = LintCode( + lintName, + 'Avoid using the bang operator. It may result in runtime exceptions.', + ); - /// Creates a new instance of [AvoidNonNullAssertionRule] - /// based on the lint configuration. - factory AvoidNonNullAssertionRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => 'Avoid using the bang operator. ' - 'It may result in runtime exceptions.', - ); + /// creates an instance of [AvoidNonNullAssertionRule] + AvoidNonNullAssertionRule() + : super( + name: lintName, + description: + 'Warns about usages of bang operator (!) except valid Map access.', + ); - return AvoidNonNullAssertionRule._(rule); - } + @override + LintCode get diagnosticCode => _code; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addPostfixExpression((node) { - if (node.operator.type != TokenType.BANG) { - return; - } - - // DCM's and Flutter's documentation treats "bang" as a valid way of - // accessing a Map. For compatibility it's excluded from this rule. - // See more: - // * https://dcm.dev/docs/rules/common/avoid-non-null-assertion - // * https://dart.dev/null-safety/understanding-null-safety#the-map-index-operator-is-nullable - final operand = node.operand; - if (operand is IndexExpression) { - final type = operand.target?.staticType; - final isInterface = type is InterfaceType; - final isMap = isInterface && - (type.isDartCoreMap || - type.allSupertypes.any((v) => v.isDartCoreMap)); - - if (isMap) { - return; - } - } - - reporter.atNode(node, code); - }); + registry.addPostfixExpression(this, AvoidNonNullAssertionVisitor(this)); } } diff --git a/lib/src/lints/avoid_non_null_assertion/visitors/avoid_non_null_assertion_visitor.dart b/lib/src/lints/avoid_non_null_assertion/visitors/avoid_non_null_assertion_visitor.dart new file mode 100644 index 00000000..1dd32ae2 --- /dev/null +++ b/lib/src/lints/avoid_non_null_assertion/visitors/avoid_non_null_assertion_visitor.dart @@ -0,0 +1,41 @@ +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/type.dart'; +import 'package:solid_lints/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart'; + +/// visitor for [AvoidNonNullAssertionRule] +class AvoidNonNullAssertionVisitor extends SimpleAstVisitor { + /// Rule associated with this visitor + final AvoidNonNullAssertionRule rule; + + /// Creates an instance of [AvoidNonNullAssertionVisitor] + AvoidNonNullAssertionVisitor(this.rule); + + @override + void visitPostfixExpression(PostfixExpression node) { + if (node.operator.type != TokenType.BANG) { + return; + } + + final operand = node.operand; + + if (operand is IndexExpression) { + final type = operand.target?.staticType; + + if (_isMap(type)) { + return; + } + } + + rule.reportAtNode(node); + } + + bool _isMap(DartType? type) { + if (type is! InterfaceType) { + return false; + } + + return type.isDartCoreMap || type.allSupertypes.any((v) => v.isDartCoreMap); + } +} 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 6bd15911..c90f2d97 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,11 +1,9 @@ -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:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:solid_lints/src/lints/avoid_returning_widgets/visitors/avoid_returning_widgets_visitor.dart'; import 'package:solid_lints/src/models/solid_lint_rule.dart'; -import 'package:solid_lints/src/utils/types_utils.dart'; /// A rule which warns about returning widgets from functions and methods. /// @@ -54,64 +52,38 @@ class AvoidReturningWidgetsRule /// This lint rule represents /// the error whether we return a widget. static const lintName = 'avoid_returning_widgets'; - static const _override = 'override'; - AvoidReturningWidgetsRule._(super.config); + static const _code = LintCode( + lintName, + 'Returning a widget from a function is considered an anti-pattern. ' + 'Unless you are overriding an existing method, ' + 'consider extracting your widget to a separate class.', + ); - /// Creates a new instance of [AvoidReturningWidgetsRule] - /// based on the lint configuration. - factory AvoidReturningWidgetsRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - paramsParser: AvoidReturningWidgetsParameters.fromJson, - problemMessage: (_) => - 'Returning a widget from a function is considered an anti-pattern. ' - 'Unless you are overriding an existing method, ' - 'consider extracting your widget to a separate class.', - ); + @override + DiagnosticCode get diagnosticCode => _code; - return AvoidReturningWidgetsRule._(rule); - } + /// Creates a new instance of [AvoidReturningWidgetsRule] + AvoidReturningWidgetsRule({ + required super.analysisOptionsLoader, + required super.parametersParser, + }) : super.withParameters( + name: _code.lowerCaseName, + description: _code.problemMessage, + ); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addDeclaration((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?)) => - type, - _ => null, - }; - - if (returnType == null) { - return; - } - - final isWidgetReturned = hasWidgetType(returnType); + super.registerNodeProcessors(registry, context); - final isIgnored = config.parameters.exclude.shouldIgnore(node); + final parameters = getParametersForContext(context) ?? + AvoidReturningWidgetsParameters.empty(); - final isOverriden = switch (node) { - FunctionDeclaration(:final functionExpression) => - functionExpression.parent is MethodDeclaration && - (functionExpression.parent! as MethodDeclaration) - .metadata - .any((m) => m.name.name == _override), - MethodDeclaration(:final metadata) => - metadata.any((m) => m.name.name == _override), - _ => false, - }; + final visitor = AvoidReturningWidgetsVisitor(this, parameters); - if (isWidgetReturned && !isOverriden && !isIgnored) { - reporter.atNode(node, code); - } - }); + registry.addCompilationUnit(this, visitor); } } diff --git a/lib/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart b/lib/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart index 7947ea64..14c27036 100644 --- a/lib/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart +++ b/lib/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart @@ -11,6 +11,13 @@ class AvoidReturningWidgetsParameters { required this.exclude, }); + /// Empty [AvoidReturningWidgetsParameters] model, excludes nothing. + factory AvoidReturningWidgetsParameters.empty() { + return AvoidReturningWidgetsParameters( + exclude: ExcludedIdentifiersListParameter(exclude: []), + ); + } + /// Method for creating from json data factory AvoidReturningWidgetsParameters.fromJson(Map json) { return AvoidReturningWidgetsParameters( diff --git a/lib/src/lints/avoid_returning_widgets/visitors/avoid_returning_widgets_visitor.dart b/lib/src/lints/avoid_returning_widgets/visitors/avoid_returning_widgets_visitor.dart new file mode 100644 index 00000000..b81f4c39 --- /dev/null +++ b/lib/src/lints/avoid_returning_widgets/visitors/avoid_returning_widgets_visitor.dart @@ -0,0 +1,84 @@ +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/type.dart'; +import 'package:solid_lints/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart'; +import 'package:solid_lints/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart'; +import 'package:solid_lints/src/utils/types_utils.dart'; + +/// A visitor that reports on functions that return widgets. +class AvoidReturningWidgetsVisitor extends RecursiveAstVisitor { + final AvoidReturningWidgetsRule _rule; + final AvoidReturningWidgetsParameters _parameters; + + /// Creates a new instance of [AvoidReturningWidgetsVisitor] + AvoidReturningWidgetsVisitor(this._rule, this._parameters); + + @override + void visitFunctionDeclaration(FunctionDeclaration node) { + super.visitFunctionDeclaration(node); + + _visitDeclaration(node); + } + + @override + void visitMethodDeclaration(MethodDeclaration node) { + super.visitMethodDeclaration(node); + + _visitDeclaration(node); + } + + @override + void visitFunctionDeclarationStatement(FunctionDeclarationStatement node) { + super.visitFunctionDeclarationStatement(node); + + _visitDeclaration(node.functionDeclaration); + } + + void _visitDeclaration(Declaration node) { + if (node is! FunctionDeclaration && node is! MethodDeclaration) { + return; + } + + final returnType = switch (node) { + Declaration( + declaredFragment: ExecutableFragment( + element: ExecutableElement(type: FunctionType(:final returnType)) + ) + ) => + returnType, + MethodDeclaration(returnType: TypeAnnotation(:final type)) => type, + FunctionDeclaration(returnType: TypeAnnotation(:final type)) => type, + _ => null, + }; + if (returnType == null) return; + + final isWidgetReturned = hasWidgetType(returnType); + if (!isWidgetReturned) return; + + final isIgnored = _parameters.exclude.shouldIgnore(node); + if (isIgnored) return; + + if (_isOverridden(node)) return; + + _rule.reportAtNode(node); + } + + bool _isOverridden(Declaration node) { + return switch (node) { + Declaration( + declaredFragment: Fragment( + element: Element( + name: final String name, + enclosingElement: final InterfaceElement enclosingElement + ) + ), + ) => + enclosingElement.getInheritedMember( + Name.forLibrary(enclosingElement.library, name), + ) != + null, + _ => false, + }; + } +} diff --git a/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart b/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart index 90cd9c7d..4068d342 100644 --- a/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart +++ b/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart @@ -1,11 +1,8 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/element/element.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:solid_lints/src/common/visitors/select_expression_identifiers_visitor.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// An `avoid_unnecessary_return_variable` rule which forbids returning /// an immutable variable if it can be rewritten in return statement itself. @@ -32,109 +29,37 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// } /// ``` /// -class AvoidUnnecessaryReturnVariableRule extends SolidLintRule { - /// This lint rule represents the error - /// when unnecessary return variable statement is found - static const lintName = 'avoid_unnecessary_return_variable'; +class AvoidUnnecessaryReturnVariableRule extends AnalysisRule { + /// The name of the lint rule. + static const _lintName = 'avoid_unnecessary_return_variable'; - AvoidUnnecessaryReturnVariableRule._(super.config); + /// The message shown when the lint is triggered. + static const String _lintMessage = """ +Avoid creating unnecessary variable only for return. +Rewrite the variable evaluation into return statement instead."""; + + /// Lint code. + static const LintCode _code = LintCode( + _lintName, + _lintMessage, + ); /// Creates a new instance of [AvoidUnnecessaryReturnVariableRule] - /// based on the lint configuration. - factory AvoidUnnecessaryReturnVariableRule.createRule( - CustomLintConfigs configs, - ) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => """ -Avoid creating unnecessary variable only for return. -Rewrite the variable evaluation into return statement instead.""", - ); + AvoidUnnecessaryReturnVariableRule() + : super( + name: _lintName, + description: _lintMessage, + ); - return AvoidUnnecessaryReturnVariableRule._(rule); - } + @override + LintCode get diagnosticCode => _code; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addReturnStatement( - (statement) { - _checkReturnStatement( - statement: statement, - reporter: reporter, - context: context, - ); - }, - ); - } - - void _checkReturnStatement({ - required ReturnStatement statement, - required DiagnosticReporter reporter, - required CustomLintContext context, - }) { - final expr = statement.expression; - if (expr is! SimpleIdentifier) return; - - final element = expr.element; - if (element is! LocalVariableElement) return; - final returnVariableElement = element; - - if (!returnVariableElement.isFinal && !returnVariableElement.isConst) { - return; - } - - final blockBody = statement.parent; - if (blockBody == null) return; - - final visitor = AvoidUnnecessaryReturnVariableVisitor( - returnVariableElement, - statement, - ); - blockBody.visitChildren(visitor); - - if (!visitor.hasBadStatementCount()) return; - - //it is 100% bad if return statement follows declaration - if (!visitor.foundTokensBetweenDeclarationAndReturn) { - reporter.atNode(statement, code); - return; - } - - final declaration = visitor.variableDeclaration; - - //check if immutable - final initializer = declaration?.initializer; - if (initializer == null) return; - - if (!_isExpressionImmutable(initializer)) return; - - reporter.atNode(statement, code); - } - - bool _isExpressionImmutable(Expression expr) { - final visitor = SelectExpressionIdentifiersVisitor(); - expr.accept(visitor); - - return visitor.identifiers.every(_isSimpleIdentifierImmutable); - } - - bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) { - switch (identifier.element) { - case final VariableElement variable: - return variable.isFinal || variable.isConst; - - case ClassElement _: - return true; - - case GetterElement(:final PropertyInducingElement variable): - return variable.isFinal || variable.isConst; - } - - return false; + final visitor = AvoidUnnecessaryReturnVariableVisitor(this); + registry.addReturnStatement(this, visitor); } } diff --git a/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart b/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart index bbc7455d..ebfad8e4 100644 --- a/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart +++ b/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart @@ -1,79 +1,70 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element.dart'; -import 'package:collection/collection.dart'; +import 'package:solid_lints/src/common/visitors/select_expression_identifiers_visitor.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart'; -/// This visitor is searches all uses of a single local variable, -/// including variable declaration. -class AvoidUnnecessaryReturnVariableVisitor extends RecursiveAstVisitor { - /// The problem expects that exactly 1 mention of return variable. - /// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier. - /// Any other amount of variable mentions implies that it is used somewhere - /// except return, so its existence is justified. - static const _badStatementCount = 1; +/// Visitor for [AvoidUnnecessaryReturnVariableRule]. +class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor { + /// The rule associated with this visitor. + final AvoidUnnecessaryReturnVariableRule _rule; - final LocalVariableElement _returnVariableElement; + /// Creates a new instance of [AvoidUnnecessaryReturnVariableVisitor]. + AvoidUnnecessaryReturnVariableVisitor(this._rule); - bool _foundTokensBetweenDeclarationAndReturn = false; - VariableDeclaration? _variableDeclaration; - int _variableStatementCounter = 0; + @override + void visitReturnStatement(ReturnStatement node) { + final expr = node.expression; + if (expr is! SimpleIdentifier) return; - final ReturnStatement _returnStatement; + final element = expr.element; + if (element is! LocalVariableElement) return; - /// After visiting holds info about whether there are any tokens - /// between variable declaration and return statement - bool get foundTokensBetweenDeclarationAndReturn => - _foundTokensBetweenDeclarationAndReturn; + if (!element.isFinal && !element.isConst) return; - /// Returns statement of local variable declaration - VariableDeclaration? get variableDeclaration => _variableDeclaration; + final block = node.parent; + if (block == null) return; - /// Creates a new instance of [AvoidUnnecessaryReturnVariableVisitor]. - AvoidUnnecessaryReturnVariableVisitor( - this._returnVariableElement, - this._returnStatement, - ); + final returnVariableUsageVisitor = ReturnVariableUsageVisitor( + node, + element, + ); - /// Defines whether the variables is used in return statement only. - bool hasBadStatementCount() => - _variableStatementCounter == _badStatementCount; + block.visitChildren(returnVariableUsageVisitor); + if (!returnVariableUsageVisitor.hasBadStatementCount) return; - @override - void visitVariableDeclarationStatement(VariableDeclarationStatement node) { - if (_collectVariableDeclaration(node)) { - _checkTokensInBetween(node, _returnStatement); + //it is 100% bad if return statement follows declaration + if (!returnVariableUsageVisitor.foundTokensBetweenDeclarationAndReturn) { + _rule.reportAtNode(node); + return; } - super.visitVariableDeclarationStatement(node); - } - @override - void visitSimpleIdentifier(SimpleIdentifier node) { - if (node.element?.id == _returnVariableElement.id) { - _variableStatementCounter++; - } + final declaration = returnVariableUsageVisitor.variableDeclaration; - super.visitSimpleIdentifier(node); - } + //check if immutable + final initializer = declaration?.initializer; + if (initializer == null) return; - bool _collectVariableDeclaration(VariableDeclarationStatement node) { - final targetVariable = node.variables.variables.firstWhereOrNull( - (v) => v.declaredFragment?.element.id == _returnVariableElement.id, - ); - if (targetVariable == null) return false; + if (!_isExpressionImmutable(initializer)) return; - _variableDeclaration = targetVariable; - return true; + _rule.reportAtNode(node); } - void _checkTokensInBetween( - VariableDeclarationStatement variableDeclaration, - ReturnStatement returnStatement, - ) { - final tokenBeforeReturn = - _returnStatement.findPrevious(_returnStatement.beginToken); + bool _isExpressionImmutable(Expression expr) { + final visitor = SelectExpressionIdentifiersVisitor(); + expr.accept(visitor); - if (tokenBeforeReturn != variableDeclaration.endToken) { - _foundTokensBetweenDeclarationAndReturn = true; - } + return visitor.identifiers.every(_isSimpleIdentifierImmutable); + } + + bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) { + return switch (identifier.element) { + ClassElement _ => true, + final VariableElement variable => variable.isFinal || variable.isConst, + GetterElement(:final PropertyInducingElement variable) => + variable.isFinal || variable.isConst, + _ => false, + }; } } diff --git a/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart b/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart new file mode 100644 index 00000000..27702549 --- /dev/null +++ b/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart @@ -0,0 +1,77 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:collection/collection.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart'; + +/// Visitor for [AvoidUnnecessaryReturnVariableRule] which checks whether the +/// return variable is used somewhere except return statement and +/// whether it is immutable. +class ReturnVariableUsageVisitor extends RecursiveAstVisitor { + /// The problem expects that exactly 1 mention of return variable. + /// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier. + /// Any other amount of variable mentions implies that it is used somewhere + /// except return, so its existence is justified. + static const _badStatementCount = 1; + + final ReturnStatement _returnStatement; + final LocalVariableElement _returnVariableElement; + + /// After visiting holds the declaration of return variable + VariableDeclaration? variableDeclaration; + + /// After visiting holds info about whether there are any tokens + bool foundTokensBetweenDeclarationAndReturn = false; + + int _variableStatementCounter = 0; + + /// Defines whether the variables is used in return statement only. + bool get hasBadStatementCount => + _variableStatementCounter == _badStatementCount; + + /// Creates a new instance of [ReturnVariableUsageVisitor]. + ReturnVariableUsageVisitor( + this._returnStatement, + this._returnVariableElement, + ); + + @override + void visitVariableDeclarationStatement(VariableDeclarationStatement node) { + if (_collectVariableDeclaration(node)) { + _checkTokensInBetween(node, _returnStatement); + } + super.visitVariableDeclarationStatement(node); + } + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + if (node.element?.id == _returnVariableElement.id) { + _variableStatementCounter++; + } + + super.visitSimpleIdentifier(node); + } + + bool _collectVariableDeclaration(VariableDeclarationStatement node) { + final targetVariable = node.variables.variables.firstWhereOrNull( + (v) => v.declaredFragment?.element.id == _returnVariableElement.id, + ); + if (targetVariable == null) return false; + variableDeclaration = targetVariable; + + return true; + } + + void _checkTokensInBetween( + VariableDeclarationStatement variableDeclaration, + ReturnStatement returnStatement, + ) { + final tokenBeforeReturn = _returnStatement.findPrevious( + _returnStatement.beginToken, + ); + + if (tokenBeforeReturn != variableDeclaration.endToken) { + foundTokensBetweenDeclarationAndReturn = true; + } + } +} diff --git a/lib/src/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart b/lib/src/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart index 0773e580..afdf2fdf 100644 --- a/lib/src/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart +++ b/lib/src/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart @@ -1,8 +1,8 @@ -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/avoid_unnecessary_setstate/visitors/avoid_unnecessary_set_state_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// A rule which warns when setState is called inside initState, didUpdateWidget /// or build methods and when it's called from a sync method that is called @@ -55,37 +55,36 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// if (mounted) setState(() => foo = 'bar'); /// } /// ``` -class AvoidUnnecessarySetStateRule extends SolidLintRule { - /// The lint name of this lint rule that represents - /// the error whether we use setState in inappropriate way. - static const lintName = 'avoid_unnecessary_setstate'; +class AvoidUnnecessarySetStateRule extends AnalysisRule { + /// The name of the lint rule. + static const _lintName = 'avoid_unnecessary_setstate'; - AvoidUnnecessarySetStateRule._(super.config); + /// The message shown when the lint rule is triggered. + static const _lintMessage = 'Avoid calling unnecessary setState. ' + 'Consider changing the state directly.'; - /// Creates a new instance of [AvoidUnnecessarySetStateRule] - /// based on the lint configuration. - factory AvoidUnnecessarySetStateRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - name: lintName, - configs: configs, - problemMessage: (_) => 'Avoid calling unnecessary setState. ' - 'Consider changing the state directly.', - ); - return AvoidUnnecessarySetStateRule._(rule); - } + /// The lint code for this rule. + static const _code = LintCode( + _lintName, + _lintMessage, + ); + + /// Creates a new instance of [AvoidUnnecessarySetStateRule]. + AvoidUnnecessarySetStateRule() + : super( + name: _lintName, + description: _lintMessage, + ); + + @override + LintCode get diagnosticCode => _code; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addClassDeclaration((node) { - final visitor = AvoidUnnecessarySetStateVisitor(); - visitor.visitClassDeclaration(node); - for (final element in visitor.setStateInvocations) { - reporter.atNode(element, code); - } - }); + final visitor = AvoidUnnecessarySetStateVisitor(this); + registry.addClassDeclaration(this, visitor); } } diff --git a/lib/src/lints/avoid_unnecessary_setstate/visitors/avoid_unnecessary_set_state_visitor.dart b/lib/src/lints/avoid_unnecessary_setstate/visitors/avoid_unnecessary_set_state_visitor.dart index 5679d5e8..4b47d939 100644 --- a/lib/src/lints/avoid_unnecessary_setstate/visitors/avoid_unnecessary_set_state_visitor.dart +++ b/lib/src/lints/avoid_unnecessary_setstate/visitors/avoid_unnecessary_set_state_visitor.dart @@ -24,11 +24,12 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:collection/collection.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart'; import 'package:solid_lints/src/lints/avoid_unnecessary_setstate/visitors/avoid_unnecessary_set_state_method_visitor.dart'; import 'package:solid_lints/src/utils/types_utils.dart'; -/// AST visitor which checks if class is State, in case yes checks its methods -class AvoidUnnecessarySetStateVisitor extends RecursiveAstVisitor { +/// Visitor for [AvoidUnnecessarySetStateRule]. +class AvoidUnnecessarySetStateVisitor extends SimpleAstVisitor { static const _checkedMethods = [ 'initState', 'didUpdateWidget', @@ -36,10 +37,13 @@ class AvoidUnnecessarySetStateVisitor extends RecursiveAstVisitor { 'build', ]; + /// The rule associated with this visitor. + final AvoidUnnecessarySetStateRule _rule; + final _setStateInvocations = []; - /// Unnecessary setState invocations - Iterable get setStateInvocations => _setStateInvocations; + /// Creates an instance of [AvoidUnnecessarySetStateVisitor]. + AvoidUnnecessarySetStateVisitor(this._rule); @override void visitClassDeclaration(ClassDeclaration node) { @@ -50,9 +54,16 @@ class AvoidUnnecessarySetStateVisitor extends RecursiveAstVisitor { return; } - final methods = node.members.whereType(); - final classMethodsNames = - methods.map((declaration) => declaration.name.lexeme).toSet(); + final body = node.body; + + if (body is! BlockClassBody) { + return; + } + + final methods = body.members.whereType(); + final classMethodsNames = methods + .map((declaration) => declaration.name.lexeme) + .toSet(); final methodBodies = methods.map((declaration) => declaration.body).toSet(); final checkedMethods = methods.where(_isMethodChecked); @@ -83,6 +94,10 @@ class AvoidUnnecessarySetStateVisitor extends RecursiveAstVisitor { ) .toList(), ); + + for (final element in _setStateInvocations) { + _rule.reportAtNode(element); + } } } @@ -105,8 +120,10 @@ class AvoidUnnecessarySetStateVisitor extends RecursiveAstVisitor { return true; } - final visitor = - AvoidUnnecessarySetStateMethodVisitor(classMethodsNames, bodies); + final visitor = AvoidUnnecessarySetStateMethodVisitor( + classMethodsNames, + bodies, + ); declaration.visitChildren(visitor); final hasSetState = visitor.setStateInvocations.isNotEmpty; 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 6f95a11d..906c934b 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 @@ -1,134 +1,52 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/element/type.dart'; -import 'package:analyzer/diagnostic/diagnostic.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:analyzer/source/source_range.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; -import 'package:solid_lints/src/utils/typecast_utils.dart'; -import 'package:solid_lints/src/utils/types_utils.dart'; - -part 'fixes/avoid_unnecessary_type_assertions_fix.dart'; - -/// The name of 'is' operator -const operatorIsName = 'is'; - -/// The name of 'whereType' method -const whereTypeMethodName = 'whereType'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/visitors/unnecessary_is_expression_visitor.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/visitors/unnecessary_where_type_visitor.dart'; /// Warns about unnecessary usage of `is` and `whereType` operators. /// /// ### Example: -/// #### BAD: -/// ```dart -/// final testList = [1.0, 2.0, 3.0]; -/// final result = testList is List; // LINT -/// final negativeResult = testList is! List; // LINT -/// testList.whereType(); // LINT -/// -/// final double d = 2.0; -/// final casted = d is double; // LINT -/// ``` -/// -/// #### GOOD: -/// ```dart -/// final dynamicList = [1.0, 2.0]; -/// dynamicList.whereType(); -/// -/// final double? nullableD = 2.0; -/// // casting `Type? is Type` is allowed -/// final castedD = nullableD is double; -/// ``` -class AvoidUnnecessaryTypeAssertions extends SolidLintRule { +/// {@macro solid_lints.avoid_unnecessary_type_assertions.example_is} +/// {@macro solid_lints.avoid_unnecessary_type_assertions.example_where} +class AvoidUnnecessaryTypeAssertionsRule extends AnalysisRule { + /// The name of 'is' operator + static const operatorIsName = 'is'; + + /// The name of 'whereType' method + static const whereTypeMethodName = 'whereType'; + /// This lint rule represents /// the error whether we use bad formatted double literals. static const lintName = 'avoid_unnecessary_type_assertions'; - static const _unnecessaryIsCode = LintCode( - name: lintName, - problemMessage: "Unnecessary usage of the '$operatorIsName' operator.", + static const _unnecessaryTypeAssertionsCode = LintCode( + lintName, + "Unnecessary usage of the {0}.", ); - static const _unnecessaryWhereTypeCode = LintCode( - name: lintName, - problemMessage: "Unnecessary usage of the '$whereTypeMethodName' method.", - ); - - AvoidUnnecessaryTypeAssertions._(super.config); - - /// Creates a new instance of [AvoidUnnecessaryTypeAssertions] - /// based on the lint configuration. - factory AvoidUnnecessaryTypeAssertions.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => "Unnecessary usage of typecast operators.", - ); - - return AvoidUnnecessaryTypeAssertions._(rule); - } - @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, - ) { - context.registry.addIsExpression((node) { - if (_isUnnecessaryIsExpression(node)) { - reporter.atNode(node, _unnecessaryIsCode); - } - }); + DiagnosticCode get diagnosticCode => _unnecessaryTypeAssertionsCode; - context.registry.addMethodInvocation((node) { - if (_isUnnecessaryWhereType(node)) { - reporter.atNode(node, _unnecessaryWhereTypeCode); - } - }); - } + /// Creates a new instance of [AvoidUnnecessaryTypeAssertionsRule] + AvoidUnnecessaryTypeAssertionsRule() + : super( + name: lintName, + description: "Unnecessary usage of typecast operators.", + ); @override - List getFixes() => [_UnnecessaryTypeAssertionsFix()]; - - bool _isUnnecessaryIsExpression(IsExpression node) { - final objectType = node.expression.staticType; - final castedType = node.type.type; - - if (objectType == null || castedType == null) { - return false; - } - final typeCast = TypeCast( - source: objectType, - target: castedType, - isReversed: node.notOperator != null, - ); - return typeCast.isUnnecessaryTypeCheck; - } - - bool _isUnnecessaryWhereType(MethodInvocation node) { - if (node - case MethodInvocation( - methodName: Identifier(name: whereTypeMethodName), - target: Expression(staticType: final targetType), - realTarget: Expression(staticType: final realTargetType), - typeArguments: TypeArgumentList(arguments: final arguments), - ) - when targetType is ParameterizedType && - isIterable(realTargetType) && - arguments.isNotEmpty) { - final objectType = targetType.typeArguments.first; - final castedType = arguments.first.type; - - if (castedType == null) { - return false; - } + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + super.registerNodeProcessors(registry, context); - final typeCast = TypeCast(source: objectType, target: castedType); + final unnecessaryIsExpressionVisitor = UnnecessaryIsExpressionVisitor(this); + registry.addIsExpression(this, unnecessaryIsExpressionVisitor); - return typeCast.isUnnecessaryTypeCheck; - } else { - return false; - } + final unnecessaryWhereTypeVisitor = UnnecessaryWhereTypeVisitor(this); + registry.addMethodInvocation(this, unnecessaryWhereTypeVisitor); } } diff --git a/lib/src/lints/avoid_unnecessary_type_assertions/fixes/avoid_unnecessary_type_assertions_fix.dart b/lib/src/lints/avoid_unnecessary_type_assertions/fixes/avoid_unnecessary_type_assertions_fix.dart index 3c429e66..814520c1 100644 --- a/lib/src/lints/avoid_unnecessary_type_assertions/fixes/avoid_unnecessary_type_assertions_fix.dart +++ b/lib/src/lints/avoid_unnecessary_type_assertions/fixes/avoid_unnecessary_type_assertions_fix.dart @@ -1,50 +1,68 @@ -part of '../avoid_unnecessary_type_assertions_rule.dart'; +import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; +import 'package:analysis_server_plugin/edit/dart/dart_fix_kind_priority.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/source/source_range.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart'; /// A Quick fix for `avoid_unnecessary_type_assertions` rule /// Suggests to remove unnecessary assertions -class _UnnecessaryTypeAssertionsFix extends DartFix { +class AvoidUnnecessaryTypeAssertionsFix extends ResolvedCorrectionProducer { + static const _avoidUnnecessaryTypeAssertionsFixKind = FixKind( + 'solid_lints.fix.${AvoidUnnecessaryTypeAssertionsRule.lintName}', + DartFixKindPriority.standard, + 'Remove the unnecessary {0}', + ); + + SourceRange? _partToRemove; + + @override + List? get fixArguments => [ + if (_partToRemove case final partToRemove?) + '"${utils.getRangeText(partToRemove).trim()}"' + else + 'type assertion', + ]; + @override - void run( - CustomLintResolver resolver, - ChangeReporter reporter, - CustomLintContext context, - Diagnostic analysisError, - List others, - ) { - context.registry.addIsExpression((node) { - if (analysisError.sourceRange.intersects(node.sourceRange)) { - _addDeletion(reporter, operatorIsName, node, node.isOperator.offset); - } - }); - - context.registry.addMethodInvocation((node) { - if (analysisError.sourceRange.intersects(node.sourceRange)) { - _addDeletion( - reporter, - whereTypeMethodName, - node, - node.operator?.offset ?? node.offset, - ); - } - }); + FixKind get fixKind => _avoidUnnecessaryTypeAssertionsFixKind; + + @override + CorrectionApplicability get applicability => + CorrectionApplicability.automatically; + + /// Creates a new instance of [AvoidUnnecessaryTypeAssertionsFix] + AvoidUnnecessaryTypeAssertionsFix({required super.context}); + + @override + Future compute(ChangeBuilder builder) async { + final isExpressionNode = node.thisOrAncestorOfType(); + if (isExpressionNode != null) { + final operatorOffset = isExpressionNode.isOperator.offset - 1; + _partToRemove = _removedPartRange(isExpressionNode, operatorOffset); + } + + final whereTypeNode = node.thisOrAncestorOfType(); + if (whereTypeNode != null && _partToRemove == null) { + final operatorOffset = + whereTypeNode.operator?.offset ?? whereTypeNode.offset; + _partToRemove = _removedPartRange(whereTypeNode, operatorOffset); + } + + final partToRemove = _partToRemove; + if (partToRemove == null) return; + + await builder.addDartFileEdit( + file, + (builder) => builder.addDeletion(partToRemove), + ); } - void _addDeletion( - ChangeReporter reporter, - String itemToDelete, - Expression node, - int operatorOffset, - ) { + SourceRange _removedPartRange(Expression node, int operatorOffset) { final targetNameLength = operatorOffset - node.offset; final removedPartLength = node.length - targetNameLength; - final changeBuilder = reporter.createChangeBuilder( - message: "Remove unnecessary '$itemToDelete'", - priority: 1, - ); - - changeBuilder.addDartFileEdit((builder) { - builder.addDeletion(SourceRange(operatorOffset, removedPartLength)); - }); + return SourceRange(operatorOffset, removedPartLength); } } diff --git a/lib/src/lints/avoid_unnecessary_type_assertions/visitors/unnecessary_is_expression_visitor.dart b/lib/src/lints/avoid_unnecessary_type_assertions/visitors/unnecessary_is_expression_visitor.dart new file mode 100644 index 00000000..7fd7b1d2 --- /dev/null +++ b/lib/src/lints/avoid_unnecessary_type_assertions/visitors/unnecessary_is_expression_visitor.dart @@ -0,0 +1,63 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart'; +import 'package:solid_lints/src/utils/typecast_utils.dart'; + +/// Visitor for [AvoidUnnecessaryTypeAssertionsRule]. +/// Reports on unnecessary usage of 'is' operator. +/// +/// ### Example: +/// {@template solid_lints.avoid_unnecessary_type_assertions.example_is} +/// #### BAD: +/// ```dart +/// final testList = [1.0, 2.0, 3.0]; +/// final result = testList is List; // LINT +/// final negativeResult = testList is! List; // LINT +/// +/// final double d = 2.0; +/// final casted = d is double; // LINT +/// ``` +/// +/// #### GOOD: +/// ```dart +/// final double? nullableD = 2.0; +/// // casting `Type? is Type` is allowed +/// final castedD = nullableD is double; +/// ``` +/// {@endtemplate} +class UnnecessaryIsExpressionVisitor extends SimpleAstVisitor { + final AvoidUnnecessaryTypeAssertionsRule _rule; + + /// Creates a new instance of [UnnecessaryIsExpressionVisitor]. + UnnecessaryIsExpressionVisitor(this._rule); + + @override + void visitIsExpression(IsExpression node) { + super.visitIsExpression(node); + + if (!_isUnnecessaryIsExpression(node)) return; + + _rule.reportAtNode( + node, + arguments: [ + "'${AvoidUnnecessaryTypeAssertionsRule.operatorIsName}' operator", + ], + ); + } + + bool _isUnnecessaryIsExpression(IsExpression node) { + final objectType = node.expression.staticType; + final castedType = node.type.type; + + if (objectType == null || castedType == null) { + return false; + } + + final typeCast = TypeCast( + source: objectType, + target: castedType, + isReversed: node.notOperator != null, + ); + return typeCast.isUnnecessaryTypeCheck; + } +} diff --git a/lib/src/lints/avoid_unnecessary_type_assertions/visitors/unnecessary_where_type_visitor.dart b/lib/src/lints/avoid_unnecessary_type_assertions/visitors/unnecessary_where_type_visitor.dart new file mode 100644 index 00000000..13bca7f5 --- /dev/null +++ b/lib/src/lints/avoid_unnecessary_type_assertions/visitors/unnecessary_where_type_visitor.dart @@ -0,0 +1,74 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:collection/collection.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart'; +import 'package:solid_lints/src/utils/typecast_utils.dart'; + +/// Visitor for [AvoidUnnecessaryTypeAssertionsRule]. +/// Reports on unnecessary usage of 'whereType' method. +/// +/// ### Example: +/// {@template solid_lints.avoid_unnecessary_type_assertions.example_where} +/// #### BAD: +/// ```dart +/// final testList = [1.0, 2.0, 3.0]; +/// testList.whereType(); // LINT +/// ``` +/// +/// #### GOOD: +/// ```dart +/// final dynamicList = [1.0, 2.0]; +/// dynamicList.whereType(); +/// ``` +/// {@endtemplate} +class UnnecessaryWhereTypeVisitor extends SimpleAstVisitor { + final AvoidUnnecessaryTypeAssertionsRule _rule; + + /// Creates a new instance of [UnnecessaryWhereTypeVisitor] + UnnecessaryWhereTypeVisitor(this._rule); + + @override + void visitMethodInvocation(MethodInvocation node) { + super.visitMethodInvocation(node); + + if (!_isUnnecessaryWhereType(node)) return; + + _rule.reportAtNode( + node, + arguments: [ + "'${AvoidUnnecessaryTypeAssertionsRule.whereTypeMethodName}' method", + ], + ); + } + + bool _isUnnecessaryWhereType(MethodInvocation node) { + if (node case MethodInvocation( + methodName: Identifier( + name: AvoidUnnecessaryTypeAssertionsRule.whereTypeMethodName, + ), + target: Expression(staticType: final InterfaceType targetType), + typeArguments: TypeArgumentList(:final arguments), + ) when arguments.isNotEmpty) { + final targetIterableType = switch (targetType) { + InterfaceType(isDartCoreIterable: true) => targetType, + InterfaceType(:final allSupertypes) => allSupertypes.firstWhereOrNull( + (e) => e.isDartCoreIterable, + ), + }; + + final objectType = targetIterableType?.typeArguments.firstOrNull; + final castedType = arguments.first.type; + + if (castedType == null || objectType == null) { + return false; + } + + final typeCast = TypeCast(source: objectType, target: castedType); + + return typeCast.isUnnecessaryTypeCheck; + } + + return false; + } +} 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 0b63cc37..6bb9c2d3 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 @@ -1,50 +1,41 @@ -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/avoid_unrelated_type_assertions/visitors/avoid_unrelated_type_assertions_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// A `avoid_unrelated_type_assertions` rule which /// warns about unnecessary usage of `as` operator -class AvoidUnrelatedTypeAssertionsRule extends SolidLintRule { - /// This lint rule represents - /// the error whether we use bad formatted double literals. - static const lintName = 'avoid_unrelated_type_assertions'; +class AvoidUnrelatedTypeAssertionsRule extends AnalysisRule { + /// The name of the lint rule. + static const _lintName = 'avoid_unrelated_type_assertions'; - AvoidUnrelatedTypeAssertionsRule._(super.config); + /// The message shown when the lint rule is triggered. + static const _lintMessage = + 'Avoid unrelated "is" assertion. The result is always "{0}".'; - /// Creates a new instance of [AvoidUnrelatedTypeAssertionsRule] - /// based on the lint configuration. - factory AvoidUnrelatedTypeAssertionsRule.createRule( - CustomLintConfigs configs, - ) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => - 'Avoid unrelated "is" assertion. The result is always "{0}".', - ); + /// Lint code for this rule. + static const LintCode _code = LintCode( + _lintName, + _lintMessage, + ); - return AvoidUnrelatedTypeAssertionsRule._(rule); - } + /// Creates a new instance of [AvoidUnrelatedTypeAssertionsRule]. + AvoidUnrelatedTypeAssertionsRule() + : super( + name: _lintName, + description: _lintMessage, + ); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, - ) { - context.registry.addIsExpression((node) { - final visitor = AvoidUnrelatedTypeAssertionsVisitor(); - visitor.visitIsExpression(node); + LintCode get diagnosticCode => _code; - for (final element in visitor.expressions.entries) { - reporter.atNode( - element.key, - code, - arguments: [element.value.toString()], - ); - } - }); + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = AvoidUnrelatedTypeAssertionsVisitor(this); + registry.addIsExpression(this, visitor); } } diff --git a/lib/src/lints/avoid_unrelated_type_assertions/visitors/avoid_unrelated_type_assertions_visitor.dart b/lib/src/lints/avoid_unrelated_type_assertions/visitors/avoid_unrelated_type_assertions_visitor.dart index e0b4aa6a..2f0e33e3 100644 --- a/lib/src/lints/avoid_unrelated_type_assertions/visitors/avoid_unrelated_type_assertions_visitor.dart +++ b/lib/src/lints/avoid_unrelated_type_assertions/visitors/avoid_unrelated_type_assertions_visitor.dart @@ -26,14 +26,15 @@ import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:collection/collection.dart'; +import 'package:solid_lints/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule.dart'; -/// AST Visitor which finds all is expressions and checks if they are -/// unrelated (result always false) -class AvoidUnrelatedTypeAssertionsVisitor extends RecursiveAstVisitor { - final _expressions = {}; +/// Visitor for [AvoidUnrelatedTypeAssertionsRule]. +class AvoidUnrelatedTypeAssertionsVisitor extends SimpleAstVisitor { + /// The rule associated with this visitor. + final AvoidUnrelatedTypeAssertionsRule _rule; - /// Map of unrelated type checks and their results - Map get expressions => _expressions; + /// Creates an instance of [AvoidUnrelatedTypeAssertionsVisitor]. + AvoidUnrelatedTypeAssertionsVisitor(this._rule); @override void visitIsExpression(IsExpression node) { @@ -47,7 +48,10 @@ class AvoidUnrelatedTypeAssertionsVisitor extends RecursiveAstVisitor { final objectType = node.expression.staticType; if (_isUnrelatedTypeCheck(objectType, castedType)) { - _expressions[node] = node.notOperator != null; + _rule.reportAtNode( + node, + arguments: [if (node.notOperator != null) 'true' else 'false'], + ); } } @@ -64,10 +68,14 @@ class AvoidUnrelatedTypeAssertionsVisitor extends RecursiveAstVisitor { return false; } - final objectCastedType = - _foundCastedTypeInObjectTypeHierarchy(objectType, castedType); - final castedObjectType = - _foundCastedTypeInObjectTypeHierarchy(castedType, objectType); + final objectCastedType = _foundCastedTypeInObjectTypeHierarchy( + objectType, + castedType, + ); + final castedObjectType = _foundCastedTypeInObjectTypeHierarchy( + castedType, + objectType, + ); if (objectCastedType == null && castedObjectType == null) { return true; } @@ -94,8 +102,8 @@ class AvoidUnrelatedTypeAssertionsVisitor extends RecursiveAstVisitor { final correctObjectType = objectType is InterfaceType && objectType.isDartAsyncFutureOr - ? objectType.typeArguments.first - : objectType; + ? objectType.typeArguments.first + : objectType; if ((correctObjectType.element == castedType.element) || castedType is DynamicType || @@ -105,8 +113,9 @@ class AvoidUnrelatedTypeAssertionsVisitor extends RecursiveAstVisitor { } if (correctObjectType is InterfaceType) { - return correctObjectType.allSupertypes - .firstWhereOrNull((value) => value.element == castedType.element); + return correctObjectType.allSupertypes.firstWhereOrNull( + (value) => value.element == castedType.element, + ); } return null; diff --git a/lib/src/lints/double_literal_format/double_literal_format_rule.dart b/lib/src/lints/double_literal_format/double_literal_format_rule.dart index d472d04c..d90d3726 100644 --- a/lib/src/lints/double_literal_format/double_literal_format_rule.dart +++ b/lib/src/lints/double_literal_format/double_literal_format_rule.dart @@ -1,17 +1,14 @@ -import 'package:analyzer/diagnostic/diagnostic.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:analyzer/source/source_range.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; - -part 'double_literal_format_utils.dart'; -part 'fixes/double_literal_format_fix.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:solid_lints/src/lints/double_literal_format/visitors/double_literal_format_visitor.dart'; /// A `double_literal_format` rule which /// checks that double literals should begin with 0. instead of just ., /// and should not end with a trailing 0. /// +/// {@template solid_lints.double_literal_format.example} /// ### Example /// /// #### BAD: @@ -25,75 +22,105 @@ part 'fixes/double_literal_format_fix.dart'; /// ```dart /// var a = 5.23, b = 0.16e+5, c = -0.25, d = -0.4e-5; /// ``` -class DoubleLiteralFormatRule extends SolidLintRule { +/// {@endtemplate} +class DoubleLiteralFormatRule extends MultiAnalysisRule { /// This lint rule represents /// the error whether we use bad formatted double literals. static const lintName = 'double_literal_format'; // Use different messages for different issues - /// This lint rule represents - /// the error whether we use double literals with a redundant leading 0. - static const _leadingZeroCode = LintCode( - name: lintName, - problemMessage: "Double literals shouldn't have redundant leading `0`.", + /// Reported when the double literal has a redundant leading 0 + /// + /// ### Example + /// + /// #### BAD: + /// + /// ```dart + /// var a = 05.23; + /// ``` + /// + /// #### GOOD: + /// + /// ```dart + /// var a = 5.23; + /// ``` + static const leadingZeroCode = LintCode( + lintName, + "Double literals shouldn't have redundant leading `0`.", correctionMessage: "Remove redundant leading `0`.", + uniqueName: 'leadingZero', ); - /// This lint rule represents - /// the error whether we use double literals with a leading decimal point. - static const _leadingDecimalCode = LintCode( - name: lintName, - problemMessage: - "Double literals shouldn't begin with the decimal point `.`.", + /// Reported when the double literal has a leading decimal point + /// without a zero before it. + /// + /// ### Example + /// + /// #### BAD: + /// + /// ```dart + /// var a = .23; + /// ``` + /// + /// #### GOOD: + /// + /// ```dart + /// var a = 0.23; + /// ``` + static const leadingDecimalCode = LintCode( + lintName, + "Double literals shouldn't begin with the decimal point `.`.", correctionMessage: "Add missing leading `0`.", + uniqueName: 'leadingDecimal', ); - /// This lint rule represents - /// the error whether we use double literals with a trailing 0. - static const _trailingZeroCode = LintCode( - name: lintName, - problemMessage: "Double literals should not end with a trailing `0`.", + /// Reported when the double literal has a redundant trailing 0. + /// + /// ### Example + /// + /// #### BAD: + /// + /// ```dart + /// var a = 5.230; + /// ``` + /// + /// #### GOOD: + /// + /// ```dart + /// var a = 5.23; + /// ``` + static const trailingZeroCode = LintCode( + lintName, + "Double literals should not end with a trailing `0`.", correctionMessage: "Remove redundant trailing `0`.", + uniqueName: 'trailingZero', ); - DoubleLiteralFormatRule._(super.config); + @override + List get diagnosticCodes => [ + leadingZeroCode, + leadingDecimalCode, + trailingZeroCode, + ]; /// Creates a new instance of [DoubleLiteralFormatRule] - /// based on the lint configuration. - factory DoubleLiteralFormatRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => 'Double literal formatting issue', - ); - - return DoubleLiteralFormatRule._(rule); - } + DoubleLiteralFormatRule() + : super( + name: lintName, + description: + 'Double literals should begin with `0.` instead of just `.`, ' + 'should not end with a trailing 0 and ' + 'should not start with a leading 0', + ); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addDoubleLiteral((node) { - final lexeme = node.literal.lexeme; + super.registerNodeProcessors(registry, context); - if (lexeme.hasLeadingZero) { - reporter.atNode(node, _leadingZeroCode); - return; - } - if (lexeme.hasLeadingDecimalPoint) { - reporter.atNode(node, _leadingDecimalCode); - return; - } - if (lexeme.hasTrailingZero) { - reporter.atNode(node, _trailingZeroCode); - return; - } - }); + final visitor = DoubleLiteralFormatVisitor(this); + registry.addDoubleLiteral(this, visitor); } - - @override - List getFixes() => [_DoubleLiteralFormatFix()]; } diff --git a/lib/src/lints/double_literal_format/double_literal_format_utils.dart b/lib/src/lints/double_literal_format/double_literal_format_utils.dart index 99e7166e..9b9d75b6 100644 --- a/lib/src/lints/double_literal_format/double_literal_format_utils.dart +++ b/lib/src/lints/double_literal_format/double_literal_format_utils.dart @@ -1,7 +1,8 @@ -part of 'double_literal_format_rule.dart'; +import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart'; -/// Useful extensions for double literals representation -extension _StringDoubleEx on String { +/// Extension to quickly check double literal formatting according to +/// [DoubleLiteralFormatRule]. +extension DoubleLiteralFormatUtils on String { /// Returns true if a double literal starts with 00 bool get hasLeadingZero => startsWith('0') && this[1] != '.'; @@ -10,7 +11,7 @@ extension _StringDoubleEx on String { /// Returns true if a mantissa of a double literal ends with 0 bool get hasTrailingZero { - final mantissa = split('e').first; + final mantissa = toLowerCase().split('e').first; return mantissa.contains('.') && mantissa.endsWith('0') && diff --git a/lib/src/lints/double_literal_format/fixes/double_literal_format_fix.dart b/lib/src/lints/double_literal_format/fixes/double_literal_format_fix.dart index b3b5af12..9b7e0426 100644 --- a/lib/src/lints/double_literal_format/fixes/double_literal_format_fix.dart +++ b/lib/src/lints/double_literal_format/fixes/double_literal_format_fix.dart @@ -1,44 +1,62 @@ -part of '../double_literal_format_rule.dart'; +import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; +import 'package:analysis_server_plugin/edit/dart/dart_fix_kind_priority.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart'; +import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_utils.dart'; -/// A Quick fix for `double_literal_format` rule +/// A Quick fix for [DoubleLiteralFormatRule] rule /// Suggests the correct value for an issue -class _DoubleLiteralFormatFix extends DartFix { +class DoubleLiteralFormatFix extends ParsedCorrectionProducer { + static const _doubleLiteralFormatKind = FixKind( + 'solid_lints.fix.${DoubleLiteralFormatRule.lintName}', + DartFixKindPriority.standard, + "Fix double literal format", + ); + + /// Creates a new instance of [DoubleLiteralFormatFix]. + DoubleLiteralFormatFix({required super.context}); + + @override + FixKind get fixKind => _doubleLiteralFormatKind; + @override - void run( - CustomLintResolver resolver, - ChangeReporter reporter, - CustomLintContext context, - Diagnostic analysisError, - List others, - ) { - context.registry.addDoubleLiteral((node) { - // checks that the literal declaration is where our warning is located - if (!analysisError.sourceRange.intersects(node.sourceRange)) return; + FixKind get multiFixKind => const FixKind( + 'solid_lints.fix.multi.${DoubleLiteralFormatRule.lintName}', + DartFixKindPriority.standard, + "Fix double literal format across files", + ); - final lexeme = node.literal.lexeme; - String? correctLexeme; + @override + CorrectionApplicability get applicability => + CorrectionApplicability.automatically; + + @override + Future compute(ChangeBuilder builder) async { + final doubleLiteralNode = node; + if (doubleLiteralNode is! DoubleLiteral) return; - if (lexeme.hasLeadingZero) { - correctLexeme = _correctLeadingZeroLexeme(lexeme); - } else if (lexeme.hasLeadingDecimalPoint) { - correctLexeme = _correctLeadingDecimalPointLexeme(lexeme); - } else if (lexeme.hasTrailingZero) { - correctLexeme = _correctTrailingZeroLexeme(lexeme); - } + final lexeme = doubleLiteralNode.literal.lexeme; + if (!lexeme.hasLeadingZero && + !lexeme.hasLeadingDecimalPoint && + !lexeme.hasTrailingZero) { + return; + } - if (correctLexeme != null) { - final changeBuilder = reporter.createChangeBuilder( - message: 'Replace by $correctLexeme', - priority: 1, - ); + final correctLexeme = _correctTrailingZeroLexeme( + _correctLeadingZeroLexeme( + _correctLeadingDecimalPointLexeme( + lexeme, + ), + ), + ); - changeBuilder.addDartFileEdit((builder) { - builder.addSimpleReplacement( - SourceRange(node.offset, node.length), - correctLexeme!, - ); - }); - } + await builder.addDartFileEdit(file, (builder) { + builder.addSimpleReplacement( + doubleLiteralNode.sourceRange, + correctLexeme, + ); }); } @@ -46,19 +64,21 @@ class _DoubleLiteralFormatFix extends DartFix { ? lexeme : _correctLeadingZeroLexeme(lexeme.substring(1)); - String _correctLeadingDecimalPointLexeme(String lexeme) => '0$lexeme'; + String _correctLeadingDecimalPointLexeme(String lexeme) => + lexeme.hasLeadingDecimalPoint ? '0$lexeme' : lexeme; String _correctTrailingZeroLexeme(String lexeme) { if (!lexeme.hasTrailingZero) { return lexeme; - } else { - final mantissa = lexeme.split('e').first; - return _correctTrailingZeroLexeme( - lexeme.replaceFirst( - mantissa, - mantissa.substring(0, mantissa.length - 1), - ), - ); } + + final mantissa = lexeme.toLowerCase().split('e').first; + + return _correctTrailingZeroLexeme( + lexeme.replaceFirst( + mantissa, + mantissa.substring(0, mantissa.length - 1), + ), + ); } } diff --git a/lib/src/lints/double_literal_format/visitors/double_literal_format_visitor.dart b/lib/src/lints/double_literal_format/visitors/double_literal_format_visitor.dart new file mode 100644 index 00000000..fa6e4e96 --- /dev/null +++ b/lib/src/lints/double_literal_format/visitors/double_literal_format_visitor.dart @@ -0,0 +1,46 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart' + show DoubleLiteralFormatRule; +import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_utils.dart'; + +/// A visitor that checks that double literals are formatted according to +/// [DoubleLiteralFormatRule]. +/// {@macro solid_lints.double_literal_format.example} +class DoubleLiteralFormatVisitor extends SimpleAstVisitor { + final DoubleLiteralFormatRule _rule; + + /// Creates a new instance of [DoubleLiteralFormatVisitor]. + DoubleLiteralFormatVisitor(this._rule); + + @override + void visitDoubleLiteral(DoubleLiteral node) { + super.visitDoubleLiteral(node); + + final lexeme = node.literal.lexeme; + + if (lexeme.hasLeadingZero) { + _rule.reportAtNode( + node, + diagnosticCode: DoubleLiteralFormatRule.leadingZeroCode, + ); + return; + } + + if (lexeme.hasLeadingDecimalPoint) { + _rule.reportAtNode( + node, + diagnosticCode: DoubleLiteralFormatRule.leadingDecimalCode, + ); + return; + } + + if (lexeme.hasTrailingZero) { + _rule.reportAtNode( + node, + diagnosticCode: DoubleLiteralFormatRule.trailingZeroCode, + ); + return; + } + } +} diff --git a/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart b/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart index a8e7c89e..0aa33e1b 100644 --- a/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart +++ b/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart @@ -1,9 +1,8 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart'; -import 'package:solid_lints/src/lints/function_lines_of_code/visitors/function_lines_of_code_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart'; import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// An approximate metric of meaningful lines of source code inside a function, @@ -12,90 +11,50 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// custom_lint: -/// rules: -/// - function_lines_of_code: -/// max_lines: 100 -/// excludeNames: -/// - "Build" +/// plugins: +/// solid_lints: +/// diagnostics: +/// function_lines_of_code: +/// max_lines: 100 +/// exclude: +/// - "Build" /// ``` class FunctionLinesOfCodeRule extends SolidLintRule { - /// This lint rule represents the error if number of - /// parameters reaches the maximum value. + /// This lint rule name. static const lintName = 'function_lines_of_code'; - FunctionLinesOfCodeRule._(super.config); - - /// Creates a new instance of [FunctionLinesOfCodeRule] - /// based on the lint configuration. - factory FunctionLinesOfCodeRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - paramsParser: FunctionLinesOfCodeParameters.fromJson, - problemMessage: (value) => - 'The maximum allowed number of lines is ${value.maxLines}. ' - 'Try splitting this function into smaller parts.', - ); - - return FunctionLinesOfCodeRule._(rule); - } + static const _code = LintCode( + lintName, + 'The maximum allowed number of lines is {0}. ' + 'Try splitting this function into smaller parts.', + ); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, - ) { - void checkNode(AstNode node) => _checkNode(resolver, reporter, node); + DiagnosticCode get diagnosticCode => _code; - void checkDeclarationNode(Declaration node) { - final isIgnored = config.parameters.exclude.shouldIgnore(node); - if (isIgnored) { - return; - } - checkNode(node); - } - - // Check for an anonymous function - void checkFunctionExpressionNode(FunctionExpression node) { - // If a FunctionExpression is an immediate child of a FunctionDeclaration - // this means it's a named function, which are already check as part of - // addFunctionDeclaration call. - if (node.parent is FunctionDeclaration) { - return; - } - checkNode(node); - } - - context.registry.addFunctionDeclaration(checkDeclarationNode); - context.registry.addMethodDeclaration(checkDeclarationNode); - context.registry.addFunctionExpression(checkFunctionExpressionNode); - } + /// Creates a new instance of [FunctionLinesOfCodeRule] + FunctionLinesOfCodeRule({ + required super.analysisOptionsLoader, + required super.parametersParser, + }) : super.withParameters( + name: lintName, + description: _code.problemMessage, + ); - void _checkNode( - CustomLintResolver resolver, - DiagnosticReporter reporter, - AstNode node, + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - final visitor = FunctionLinesOfCodeVisitor(resolver.lineInfo); - node.visitChildren(visitor); + super.registerNodeProcessors(registry, context); - if (visitor.linesWithCode.length > config.parameters.maxLines) { - if (node is! AnnotatedNode) { - reporter.atNode(node, code); - return; - } + final parameters = + getParametersForContext(context) ?? + FunctionLinesOfCodeParameters.empty(); - final startOffset = node.firstTokenAfterCommentAndMetadata.offset; - final lengthDifference = startOffset - node.offset; + final visitor = FunctionLinesOfCodeRuleVisitor(this, context, parameters); - reporter.atOffset( - offset: startOffset, - length: node.length - lengthDifference, - diagnosticCode: code, - ); - } + registry.addCompilationUnit(this, visitor); } } 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 7df6da2a..3e748c49 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 @@ -18,6 +18,14 @@ class FunctionLinesOfCodeParameters { required this.exclude, }); + /// Empty [FunctionLinesOfCodeParameters] model with default max lines. + factory FunctionLinesOfCodeParameters.empty() { + return FunctionLinesOfCodeParameters( + maxLines: _defaultMaxLines, + exclude: ExcludedIdentifiersListParameter(exclude: []), + ); + } + /// Method for creating from json data factory FunctionLinesOfCodeParameters.fromJson(Map json) => FunctionLinesOfCodeParameters( diff --git a/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart b/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart new file mode 100644 index 00000000..e1901435 --- /dev/null +++ b/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart @@ -0,0 +1,76 @@ +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/visitors/function_lines_of_code_visitor.dart'; + +/// A visitor that reports on functions/methods exceeding the max line limit. +class FunctionLinesOfCodeRuleVisitor extends RecursiveAstVisitor { + final FunctionLinesOfCodeRule _rule; + final RuleContext _context; + final FunctionLinesOfCodeParameters _parameters; + + /// Creates a new instance of [FunctionLinesOfCodeRuleVisitor]. + FunctionLinesOfCodeRuleVisitor(this._rule, this._context, this._parameters); + + @override + void visitFunctionDeclaration(FunctionDeclaration node) { + final isIgnored = _parameters.exclude.shouldIgnore(node); + if (!isIgnored) { + _checkNode(node); + } + super.visitFunctionDeclaration(node); + } + + @override + void visitMethodDeclaration(MethodDeclaration node) { + final isIgnored = _parameters.exclude.shouldIgnore(node); + if (!isIgnored) { + _checkNode(node); + } + super.visitMethodDeclaration(node); + } + + @override + void visitFunctionExpression(FunctionExpression node) { + super.visitFunctionExpression(node); + + if (node.parent is FunctionDeclaration) { + return; + } + _checkNode(node); + } + + void _checkNode(AstNode node) { + final lineInfo = _context.currentUnit?.unit.lineInfo; + if (lineInfo == null) return; + + final visitor = FunctionLinesOfCodeVisitor(lineInfo); + node.visitChildren(visitor); + + if (visitor.linesWithCode.length > _parameters.maxLines) { + final reporter = _context.currentUnit?.diagnosticReporter; + if (reporter == null) return; + + if (node is! AnnotatedNode) { + reporter.atNode( + node, + _rule.diagnosticCode, + arguments: [_parameters.maxLines], + ); + return; + } + + final startOffset = node.firstTokenAfterCommentAndMetadata.offset; + final lengthDifference = startOffset - node.offset; + + reporter.atOffset( + offset: startOffset, + length: node.length - lengthDifference, + diagnosticCode: _rule.diagnosticCode, + arguments: [_parameters.maxLines], + ); + } + } +} diff --git a/lib/src/lints/newline_before_return/newline_before_return_rule.dart b/lib/src/lints/newline_before_return/newline_before_return_rule.dart index 3624a7c6..68e8360e 100644 --- a/lib/src/lints/newline_before_return/newline_before_return_rule.dart +++ b/lib/src/lints/newline_before_return/newline_before_return_rule.dart @@ -21,11 +21,11 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/newline_before_return/visitors/newline_before_return_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; // Inspired by ESLint (https://eslint.org/docs/rules/newline-before-return) @@ -72,38 +72,35 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// } /// } /// ``` -class NewlineBeforeReturnRule extends SolidLintRule { - /// This lint rule represents the error if - /// newline is missing before return statement - static const String lintName = 'newline_before_return'; +class NewlineBeforeReturnRule extends AnalysisRule { + /// The name of the lint rule. + static const String _lintName = 'newline_before_return'; - NewlineBeforeReturnRule._(super.config); + /// The message shown when the lint is triggered. + static const String _lintMessage = 'Missing blank line before return.'; - /// Creates a new instance of [NewlineBeforeReturnRule] - /// based on the lint configuration. - factory NewlineBeforeReturnRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (value) => "Missing blank line before return.", - ); + /// Lint code for this rule. + static const LintCode _code = LintCode( + _lintName, + _lintMessage, + ); - return NewlineBeforeReturnRule._(rule); - } + /// Creates a new instance of [NewlineBeforeReturnRule]. + NewlineBeforeReturnRule() + : super( + name: _lintName, + description: _lintMessage, + ); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, - ) { - context.registry.addReturnStatement((node) { - final visitor = NewLineBeforeReturnVisitor(resolver.lineInfo); - visitor.visitReturnStatement(node); + LintCode get diagnosticCode => _code; - for (final element in visitor.statements) { - reporter.atNode(element, code); - } - }); + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = NewLineBeforeReturnVisitor(this); + registry.addReturnStatement(this, visitor); } } diff --git a/lib/src/lints/newline_before_return/visitors/newline_before_return_visitor.dart b/lib/src/lints/newline_before_return/visitors/newline_before_return_visitor.dart index cad8b50a..b2e74cdc 100644 --- a/lib/src/lints/newline_before_return/visitors/newline_before_return_visitor.dart +++ b/lib/src/lints/newline_before_return/visitors/newline_before_return_visitor.dart @@ -25,17 +25,14 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/source/line_info.dart'; +import 'package:solid_lints/src/lints/newline_before_return/newline_before_return_rule.dart'; -/// The AST visitor that will all return statements. -class NewLineBeforeReturnVisitor extends RecursiveAstVisitor { - final LineInfo _lineInfo; - final _statements = []; +/// Visitor for [NewlineBeforeReturnRule]. +class NewLineBeforeReturnVisitor extends SimpleAstVisitor { + final NewlineBeforeReturnRule _rule; /// Creates instance of [NewLineBeforeReturnVisitor] with line info - NewLineBeforeReturnVisitor(this._lineInfo); - - /// List of all return statements - Iterable get statements => _statements; + NewLineBeforeReturnVisitor(this._rule); @override void visitReturnStatement(ReturnStatement node) { @@ -43,9 +40,9 @@ class NewLineBeforeReturnVisitor extends RecursiveAstVisitor { if (!_statementIsInBlock(node)) return; if (_statementIsFirstInBlock(node)) return; - if (_statementHasNewLineBefore(node, _lineInfo)) return; + if (_statementHasNewLineBefore(node)) return; - _statements.add(node); + _rule.reportAtNode(node); } static bool _statementIsInBlock(ReturnStatement node) => node.parent is Block; @@ -55,14 +52,22 @@ class NewLineBeforeReturnVisitor extends RecursiveAstVisitor { static bool _statementHasNewLineBefore( ReturnStatement node, - LineInfo lineInfo, ) { - final previousTokenLineNumber = - lineInfo.getLocation(node.returnKeyword.previous!.end).lineNumber; + final root = node.root; + if (root is! CompilationUnit) return true; + + final lineInfo = root.lineInfo; + final previousToken = node.returnKeyword.previous; + + if (previousToken == null) return true; + final previousTokenLineNumber = lineInfo + .getLocation(previousToken.end) + .lineNumber; final lastNotEmptyLineToken = _optimalToken(node.returnKeyword, lineInfo); - final tokenLineNumber = - lineInfo.getLocation(lastNotEmptyLineToken.offset).lineNumber; + final tokenLineNumber = lineInfo + .getLocation(lastNotEmptyLineToken.offset) + .lineNumber; return tokenLineNumber > previousTokenLineNumber + 1; } diff --git a/lib/src/lints/no_equal_then_else/no_equal_then_else_rule.dart b/lib/src/lints/no_equal_then_else/no_equal_then_else_rule.dart index eaf8e389..8099a2c8 100644 --- a/lib/src/lints/no_equal_then_else/no_equal_then_else_rule.dart +++ b/lib/src/lints/no_equal_then_else/no_equal_then_else_rule.dart @@ -1,8 +1,8 @@ -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/no_equal_then_else/visitors/no_equal_then_else_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; // Inspired by PVS-Studio (https://www.viva64.com/en/w/v6004/) @@ -41,38 +41,36 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// /// selectedValue = condition ? valueA : valueB; /// ``` -class NoEqualThenElseRule extends SolidLintRule { - /// This lint rule represents the error if - /// 'if' statements or conditional expression is redundant - static const String lintName = 'no_equal_then_else'; +class NoEqualThenElseRule extends AnalysisRule { + /// The name of the lint rule. + static const String _lintName = 'no_equal_then_else'; - NoEqualThenElseRule._(super.config); + /// The message shown when the lint is triggered. + static const String _lintMessage = 'Then and else branches are equal.'; - /// Creates a new instance of [NoEqualThenElseRule] - /// based on the lint configuration. - factory NoEqualThenElseRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (value) => "Then and else branches are equal.", - ); + /// Lint code + static const _code = LintCode( + _lintName, + _lintMessage, + ); - return NoEqualThenElseRule._(rule); - } + /// Create a new instance of [NoEqualThenElseRule] + NoEqualThenElseRule() + : super( + name: _lintName, + description: _lintMessage, + ); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, - ) { - context.registry.addCompilationUnit((node) { - final visitor = NoEqualThenElseVisitor(); - node.accept(visitor); + LintCode get diagnosticCode => _code; - for (final element in visitor.nodes) { - reporter.atNode(element, code); - } - }); + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = NoEqualThenElseVisitor(this); + registry.addIfStatement(this, visitor); + registry.addConditionalExpression(this, visitor); } } diff --git a/lib/src/lints/no_equal_then_else/visitors/no_equal_then_else_visitor.dart b/lib/src/lints/no_equal_then_else/visitors/no_equal_then_else_visitor.dart index 0ec18307..98b90230 100644 --- a/lib/src/lints/no_equal_then_else/visitors/no_equal_then_else_visitor.dart +++ b/lib/src/lints/no_equal_then_else/visitors/no_equal_then_else_visitor.dart @@ -23,14 +23,15 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/no_equal_then_else/no_equal_then_else_rule.dart'; -/// The AST visitor that will collect all unnecessary if statements and -/// conditional expressions. -class NoEqualThenElseVisitor extends RecursiveAstVisitor { - final _nodes = []; +/// Visitor for [NoEqualThenElseRule]. +class NoEqualThenElseVisitor extends SimpleAstVisitor { + /// The rule associated with this visitor. + final NoEqualThenElseRule _rule; - /// All unnecessary if statements and conditional expressions. - Iterable get nodes => _nodes; + /// Creates an instance of [NoEqualThenElseVisitor] + NoEqualThenElseVisitor(this._rule); @override void visitIfStatement(IfStatement node) { @@ -39,7 +40,7 @@ class NoEqualThenElseVisitor extends RecursiveAstVisitor { if (node.elseStatement != null && node.elseStatement is! IfStatement && node.thenStatement.toString() == node.elseStatement.toString()) { - _nodes.add(node); + _rule.reportAtNode(node); } } @@ -48,7 +49,7 @@ class NoEqualThenElseVisitor extends RecursiveAstVisitor { super.visitConditionalExpression(node); if (node.thenExpression.toString() == node.elseExpression.toString()) { - _nodes.add(node); + _rule.reportAtNode(node); } } } diff --git a/lib/src/lints/prefer_early_return/prefer_early_return_rule.dart b/lib/src/lints/prefer_early_return/prefer_early_return_rule.dart index e4b54b18..582378df 100644 --- a/lib/src/lints/prefer_early_return/prefer_early_return_rule.dart +++ b/lib/src/lints/prefer_early_return/prefer_early_return_rule.dart @@ -1,8 +1,8 @@ -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// A rule which highlights `if` statements that span the entire body, /// and suggests replacing them with a reversed boolean check @@ -31,38 +31,36 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// c; /// } /// ``` -class PreferEarlyReturnRule extends SolidLintRule { - /// This lint rule represents the error if - /// 'if' statements should be reversed +class PreferEarlyReturnRule extends AnalysisRule { + /// Lint name static const String lintName = 'prefer_early_return'; - PreferEarlyReturnRule._(super.config); + /// Lint code + static const LintCode _code = LintCode( + lintName, + "Use reverse if to reduce nesting", + ); - /// Creates a new instance of [PreferEarlyReturnRule] - /// based on the lint configuration. - factory PreferEarlyReturnRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => "Use reverse if to reduce nesting", - ); + /// Creates an instance of [PreferEarlyReturnRule] + PreferEarlyReturnRule() + : super( + name: lintName, + description: 'Use reverse if to reduce nesting', + ); - return PreferEarlyReturnRule._(rule); - } + @override + LintCode get diagnosticCode => _code; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addBlockFunctionBody((node) { - final visitor = PreferEarlyReturnVisitor(); - node.accept(visitor); + final visitor = PreferEarlyReturnVisitor( + rule: this, + context: context, + ); - for (final element in visitor.nodes) { - reporter.atNode(element, code); - } - }); + registry.addBlockFunctionBody(this, visitor); } } diff --git a/lib/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart b/lib/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart index 127de6e8..eb30616c 100644 --- a/lib/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart +++ b/lib/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart @@ -1,14 +1,23 @@ +import 'package:analyzer/analysis_rule/rule_context.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_rule.dart'; import 'package:solid_lints/src/lints/prefer_early_return/visitors/return_statement_visitor.dart'; import 'package:solid_lints/src/lints/prefer_early_return/visitors/throw_expression_visitor.dart'; -/// The AST visitor that will collect all unnecessary if statements +/// Visitor for [PreferEarlyReturnRule]. class PreferEarlyReturnVisitor extends RecursiveAstVisitor { - final _nodes = []; + /// The rule associated with the visitor. + final PreferEarlyReturnRule rule; - /// All unnecessary if statements and conditional expressions. - Iterable get nodes => _nodes; + /// The context associated with the visitor. + final RuleContext context; + + /// Constructor for [PreferEarlyReturnVisitor]. + PreferEarlyReturnVisitor({ + required this.rule, + required this.context, + }); @override void visitBlockFunctionBody(BlockFunctionBody node) { @@ -16,7 +25,9 @@ class PreferEarlyReturnVisitor extends RecursiveAstVisitor { if (node.block.statements.isEmpty) return; - final (ifStatements, nextStatement) = _getStartIfStatements(node); + final (ifStatements, nextStatement) = _getIfStatementsAndNextStatement( + node, + ); if (ifStatements.isEmpty) return; // limit visitor to only work with functions @@ -27,25 +38,24 @@ class PreferEarlyReturnVisitor extends RecursiveAstVisitor { if (!nextStatementIsEmptyReturn && !nextStatementIsNull) return; - _handleIfStatement(ifStatements.last); - } + final lastIf = ifStatements.last; - void _handleIfStatement(IfStatement node) { - if (_isElseIfStatement(node)) return; - if (_hasElseStatement(node)) return; - if (_hasReturnStatement(node)) return; - if (_hasThrowExpression(node)) return; + if (lastIf case IfStatement(elseStatement: Statement())) return; + if (_hasReturnStatement(lastIf) || _hasThrowExpression(lastIf)) return; - _nodes.add(node); + context.currentUnit?.diagnosticReporter.atNode( + lastIf, + rule.diagnosticCode, + ); } -// returns a list of if statements at the start of the function -// and the next statement after it -// examples: -// [if, if, if, return] -> ([if, if, if], return) -// [if, if, if, _doSomething, return] -> ([if, if, if], _doSomething) -// [if, if, if] -> ([if, if, if], null) - (List, Statement?) _getStartIfStatements( + // returns a list of if statements at the start of the function + // and the next statement after it + // examples: + // [if, if, if, return] -> ([if, if, if], return) + // [if, if, if, _doSomething, return] -> ([if, if, if], _doSomething) + // [if, if, if] -> ([if, if, if], null) + (List, Statement?) _getIfStatementsAndNextStatement( BlockFunctionBody body, ) { final List ifStatements = []; @@ -56,15 +66,8 @@ class PreferEarlyReturnVisitor extends RecursiveAstVisitor { return (ifStatements, statement); } } - return (ifStatements, null); - } - bool _hasElseStatement(IfStatement node) { - return node.elseStatement != null; - } - - bool _isElseIfStatement(IfStatement node) { - return node.elseStatement != null && node.elseStatement is IfStatement; + return (ifStatements, null); } bool _hasReturnStatement(Statement node) { diff --git a/lib/src/lints/proper_super_calls/proper_super_calls_rule.dart b/lib/src/lints/proper_super_calls/proper_super_calls_rule.dart index 34df0dbc..fb82e8e8 100644 --- a/lib/src/lints/proper_super_calls/proper_super_calls_rule.dart +++ b/lib/src/lints/proper_super_calls/proper_super_calls_rule.dart @@ -1,8 +1,8 @@ -import 'package:analyzer/dart/ast/ast.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'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:solid_lints/src/lints/proper_super_calls/visitors/proper_super_calls_visitor.dart'; /// Ensures that `super` calls are made in the correct order for the following /// StatefulWidget methods: @@ -43,130 +43,44 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// super.dispose(); // OK /// } /// ``` -class ProperSuperCallsRule extends SolidLintRule { - /// This lint rule represents - /// the error whether the initState and dispose methods - /// are called in the incorrect order - static const lintName = 'proper_super_calls'; - static const _initState = 'initState'; - static const _dispose = 'dispose'; - static const _override = 'override'; - - /// This lint rule represents - /// the error whether super.initState() should be called first - static const _superInitStateCode = LintCode( - name: lintName, - problemMessage: "super.initState() should be first", +class ProperSuperCallsRule extends AnalysisRule { + /// Lint rule name. + static const String lintName = 'proper_super_calls'; + + /// Error code for invalid `super.initState()` placement. + static const LintCode superInitStateCode = LintCode( + lintName, + 'super.initState() should be first', ); - /// This lint rule represents - /// the error whether super.dispose() should be called last - static const _superDisposeCode = LintCode( - name: lintName, - problemMessage: "super.dispose() should be last", + /// Error code for invalid `super.dispose()` placement. + static const LintCode superDisposeCode = LintCode( + lintName, + 'super.dispose() should be last', ); - ProperSuperCallsRule._(super.config); - - /// Creates a new instance of [ProperSuperCallsRule] - /// based on the lint configuration. - factory ProperSuperCallsRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - name: lintName, - configs: configs, - problemMessage: (_) => 'Proper super calls issue', - ); + /// Creates an instance of [ProperSuperCallsRule]. + ProperSuperCallsRule() + : super( + name: lintName, + description: + 'Ensures proper ordering of Flutter lifecycle super calls.', + ); - return ProperSuperCallsRule._(rule); - } + @override + LintCode get diagnosticCode => superInitStateCode; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addMethodDeclaration( - (node) { - final methodName = node.name.toString(); - final body = node.body; - - if (methodName case _initState || _dispose - when body is BlockFunctionBody) { - final statements = body.block.statements; - - _checkSuperCalls( - node, - methodName, - statements, - reporter, - ); - } - }, + registry.addMethodDeclaration( + this, + ProperSuperCallsVisitor( + rule: this, + context: context, + ), ); } - - /// This method report an error whether `super.initState()` - /// or `super.dispose()` are called incorrect - void _checkSuperCalls( - MethodDeclaration node, - String methodName, - List statements, - DiagnosticReporter reporter, - ) { - final hasOverrideAnnotation = - node.metadata.any((annotation) => annotation.name.name == _override); - - if (!hasOverrideAnnotation) return; - if (methodName == _initState && !_isSuperInitStateCalledFirst(statements)) { - reporter.atNode( - node, - _superInitStateCode, - ); - } - if (methodName == _dispose && !_isSuperDisposeCalledLast(statements)) { - reporter.atNode( - node, - _superDisposeCode, - ); - } - } - - /// Returns `true` if `super.initState()` is called before other code in the - /// `initState` method, `false` otherwise. - bool _isSuperInitStateCalledFirst(List statements) { - if (statements.isEmpty) return false; - final firstStatement = statements.first; - - if (firstStatement is ExpressionStatement) { - final expression = firstStatement.expression; - - final isSuperInitStateCalledFirst = expression is MethodInvocation && - expression.target is SuperExpression && - expression.methodName.toString() == _initState; - - return isSuperInitStateCalledFirst; - } - - return false; - } - - /// Returns `true` if `super.dispose()` is called at the end of the `dispose` - /// method, `false` otherwise. - bool _isSuperDisposeCalledLast(List statements) { - if (statements.isEmpty) return false; - final lastStatement = statements.last; - - if (lastStatement is ExpressionStatement) { - final expression = lastStatement.expression; - - final lastStatementIsSuperDispose = expression is MethodInvocation && - expression.target is SuperExpression && - expression.methodName.toString() == _dispose; - - return lastStatementIsSuperDispose; - } - - return false; - } } diff --git a/lib/src/lints/proper_super_calls/visitors/proper_super_calls_visitor.dart b/lib/src/lints/proper_super_calls/visitors/proper_super_calls_visitor.dart new file mode 100644 index 00000000..6cbe4759 --- /dev/null +++ b/lib/src/lints/proper_super_calls/visitors/proper_super_calls_visitor.dart @@ -0,0 +1,112 @@ +import 'package:analyzer/analysis_rule/rule_context.dart'; +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/type.dart'; +import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; + +/// Visitor for [ProperSuperCallsRule]. +class ProperSuperCallsVisitor extends SimpleAstVisitor { + /// The rule associated with this visitor. + final ProperSuperCallsRule rule; + + /// The context associated with this visitor. + final RuleContext context; + + static const _initState = 'initState'; + static const _dispose = 'dispose'; + static const _flutterStateClass = 'State'; + + /// Creates a new instance of [ProperSuperCallsVisitor]. + ProperSuperCallsVisitor({ + required this.rule, + required this.context, + }); + + @override + void visitMethodDeclaration(MethodDeclaration node) { + final methodName = node.name.lexeme; + final body = node.body; + + if ((methodName != _initState && methodName != _dispose) || + body is! BlockFunctionBody) { + return; + } + + if (!_overridesFlutterStateMethod(node)) { + return; + } + + final statements = body.block.statements; + final reporter = context.currentUnit?.diagnosticReporter; + + if (reporter == null) return; + + if (methodName == _initState && + !_isSuperCallFirst(statements, _initState)) { + reporter.atToken( + node.name, + ProperSuperCallsRule.superInitStateCode, + ); + } + + if (methodName == _dispose && !_isSuperCallLast(statements, _dispose)) { + reporter.atToken( + node.name, + ProperSuperCallsRule.superDisposeCode, + ); + } + } + + bool _overridesFlutterStateMethod(MethodDeclaration node) { + final classElement = node.declaredFragment?.element.enclosingElement; + + if (classElement is! ClassElement) { + return false; + } + + final methodName = node.name.lexeme; + + final supertype = classElement.supertype; + + if (supertype == null) { + return false; + } + + final isStateSubclass = _isStateSubclass(supertype); + + if (!isStateSubclass) { + return false; + } + + return methodName == _initState || methodName == _dispose; + } + + bool _isStateSubclass(InterfaceType supertype) { + final isStateSubclass = supertype.element.name == _flutterStateClass || + supertype.allSupertypes.any( + (t) => t.element.name == _flutterStateClass, + ); + return isStateSubclass; + } + + bool _isSuperCallFirst(List statements, String name) { + return statements.isNotEmpty && _isTargetSuperCall(statements.first, name); + } + + bool _isSuperCallLast(List statements, String name) { + return statements.isNotEmpty && _isTargetSuperCall(statements.last, name); + } + + bool _isTargetSuperCall(Statement statement, String name) { + if (statement is! ExpressionStatement) { + return false; + } + + final expression = statement.expression; + + return expression is MethodInvocation && + expression.target is SuperExpression && + expression.methodName.name == name; + } +} diff --git a/lib/src/models/rule_config.dart b/lib/src/models/rule_config.dart deleted file mode 100644 index 35c35b96..00000000 --- a/lib/src/models/rule_config.dart +++ /dev/null @@ -1,43 +0,0 @@ -import 'package:analyzer/error/error.dart' as error; -import 'package:custom_lint_builder/custom_lint_builder.dart'; - -/// Type definition of a value factory which allows us to map data from -/// YAML configuration to an object of type [T]. -typedef RuleParameterParser = T Function(Map json); - -/// Type definition for a problem message factory after finding a problem -/// by a given lint. -typedef RuleProblemFactory = String Function(T value); - -/// [RuleConfig] allows us to quickly parse a lint rule and -/// declare basic configuration for it. -class RuleConfig { - /// Constructor for [RuleConfig] model. - RuleConfig({ - required this.name, - required CustomLintConfigs configs, - required RuleProblemFactory problemMessage, - RuleParameterParser? paramsParser, - }) : enabled = configs.rules[name]?.enabled ?? false, - parameters = paramsParser?.call(configs.rules[name]?.json ?? {}) as T, - _problemMessageFactory = problemMessage; - - /// This lint rule represents the error. - final String name; - - /// A flag which indicates whether this rule was enabled by the user. - final bool enabled; - - /// Value with a configuration for a particular rule. - final T parameters; - - /// Factory for generating error messages. - final RuleProblemFactory _problemMessageFactory; - - /// [LintCode] which is generated based on the provided data. - LintCode get lintCode => LintCode( - name: name, - problemMessage: _problemMessageFactory(parameters), - errorSeverity: error.DiagnosticSeverity.WARNING, - ); -} diff --git a/lib/src/models/solid_lint_rule.dart b/lib/src/models/solid_lint_rule.dart index a536eb4f..9f5bee56 100644 --- a/lib/src/models/solid_lint_rule.dart +++ b/lib/src/models/solid_lint_rule.dart @@ -1,16 +1,43 @@ -import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; + +/// A function that parses the rule parameters from analysis options json +typedef RuleParametersParser = T Function(Map); /// A base class for emitting information about /// issues with user's `.dart` files. -abstract class SolidLintRule extends DartLintRule { +abstract class SolidLintRule extends AnalysisRule { + final AnalysisOptionsLoader? _analysisOptionsLoader; + + final RuleParametersParser? _parametersParser; + /// Constructor for [SolidLintRule] model. - SolidLintRule(this.config) : super(code: config.lintCode); + SolidLintRule({ + required super.name, + required super.description, + super.state, + }) : _analysisOptionsLoader = null, + _parametersParser = null; + + /// Constructor for [SolidLintRule] model with parameters. + SolidLintRule.withParameters({ + required AnalysisOptionsLoader analysisOptionsLoader, + required RuleParametersParser parametersParser, + required super.name, + required super.description, + super.state, + }) : _analysisOptionsLoader = analysisOptionsLoader, + _parametersParser = parametersParser; + + /// Reads the rule parameters from analysis options and parses them to [T] + T? getParametersForContext(RuleContext context) { + _analysisOptionsLoader?.loadRulesOptionsFromContext(context); - /// Configuration for a particular rule with all the - /// defined custom parameters. - final RuleConfig config; + final unparsedParameters = + _analysisOptionsLoader?.getRuleOptions(context, name); + if (unparsedParameters == null) return null; - /// A flag which indicates whether this rule was enabled by the user. - bool get enabled => config.enabled; + return _parametersParser?.call(unparsedParameters); + } } diff --git a/lint_test/.gitignore b/lint_test/.gitignore new file mode 100644 index 00000000..3820a95c --- /dev/null +++ b/lint_test/.gitignore @@ -0,0 +1,45 @@ +# Miscellaneous +*.class +*.log +*.pyc +*.swp +.DS_Store +.atom/ +.build/ +.buildlog/ +.history +.svn/ +.swiftpm/ +migrate_working_dir/ + +# IntelliJ related +*.iml +*.ipr +*.iws +.idea/ + +# The .vscode folder contains launch configuration and tasks you configure in +# VS Code which you may wish to be included in version control, so this line +# is commented out by default. +#.vscode/ + +# Flutter/Dart/Pub related +**/doc/api/ +**/ios/Flutter/.last_build_id +.dart_tool/ +.flutter-plugins-dependencies +.pub-cache/ +.pub/ +/build/ +/coverage/ + +# Symbolication related +app.*.symbols + +# Obfuscation related +app.*.map.json + +# Android Studio will place build artifacts here +/android/app/debug +/android/app/profile +/android/app/release diff --git a/lint_test/.metadata b/lint_test/.metadata new file mode 100644 index 00000000..0bb2266c --- /dev/null +++ b/lint_test/.metadata @@ -0,0 +1,30 @@ +# This file tracks properties of this Flutter project. +# Used by Flutter tool to assess capabilities and perform upgrades etc. +# +# This file should be version controlled and should not be manually edited. + +version: + revision: "05db9689081f091050f01aed79f04dce0c750154" + channel: "stable" + +project_type: app + +# Tracks metadata for the flutter migrate command +migration: + platforms: + - platform: root + create_revision: 05db9689081f091050f01aed79f04dce0c750154 + base_revision: 05db9689081f091050f01aed79f04dce0c750154 + - platform: web + create_revision: 05db9689081f091050f01aed79f04dce0c750154 + base_revision: 05db9689081f091050f01aed79f04dce0c750154 + + # User provided section + + # List of Local paths (relative to this file) that should be + # ignored by the migrate tool. + # + # Files that are not part of the templates will be ignored by default. + unmanaged_files: + - 'lib/main.dart' + - 'ios/Runner.xcodeproj/project.pbxproj' diff --git a/lint_test/README.md b/lint_test/README.md new file mode 100644 index 00000000..9002c39f --- /dev/null +++ b/lint_test/README.md @@ -0,0 +1,3 @@ +# solid_lints_test + +A new Flutter project. diff --git a/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart b/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart deleted file mode 100644 index 9d6fa72a..00000000 --- a/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart +++ /dev/null @@ -1,58 +0,0 @@ -// ignore_for_file: unused_local_variable - -import 'package:flutter/foundation.dart'; - -/// Test the avoid_debug_print_in_release -void avoidDebugPrintTest() { - // expect_lint: avoid_debug_print_in_release - debugPrint(''); - - // expect_lint: avoid_debug_print_in_release - final test = debugPrint; - - var test2; - - // expect_lint: avoid_debug_print_in_release - test2 = debugPrint; - - test.call('test'); - - // expect_lint: avoid_debug_print_in_release - final test3 = debugPrint(''); - - someOtherFunction(); - - if (!kReleaseMode) { - debugPrint(''); - - final test = debugPrint; - - var test2; - - test2 = debugPrint; - - test.call('test'); - - final test3 = debugPrint(''); - - someOtherFunction(); - - if (true) { - debugPrint(''); - - final test = debugPrint; - - var test2; - - test2 = debugPrint; - - test.call('test'); - - final test3 = debugPrint(''); - } - } -} - -void someOtherFunction() { - print('iii'); -} diff --git a/lint_test/avoid_final_with_getter_test.dart b/lint_test/avoid_final_with_getter_test.dart deleted file mode 100644 index 4f0fa323..00000000 --- a/lint_test/avoid_final_with_getter_test.dart +++ /dev/null @@ -1,35 +0,0 @@ -// ignore_for_file: type_annotate_public_apis, prefer_match_file_name, unused_local_variable - -/// Check final private field with getter fail -/// `avoid_final_with_getter` - -class Fail { - final int _myField = 0; - - // expect_lint: avoid_final_with_getter - int get myField => _myField; -} - -class FailOtherName { - final int _myField = 0; - - // expect_lint: avoid_final_with_getter - int get myFieldInt => _myField; -} - -class FailStatic { - static final int _myField = 0; - - // expect_lint: avoid_final_with_getter - static int get myField => _myField; -} - -class Skip { - final int _myField = 0; - - int get myField => _myField + 1; // it is not a getter for the field -} - -class Good { - final int myField = 0; -} diff --git a/lint_test/avoid_global_state_test.dart b/lint_test/avoid_global_state_test.dart deleted file mode 100644 index 37028d42..00000000 --- a/lint_test/avoid_global_state_test.dart +++ /dev/null @@ -1,30 +0,0 @@ -// ignore_for_file: type_annotate_public_apis, prefer_match_file_name, unused_local_variable - -/// Check global mutable variable fail -/// `avoid_global_state` - -// expect_lint: avoid_global_state -var globalMutable = 0; - -final globalFinal = 1; - -const globalConst = 1; - -class Test { - static final int globalFinal = 1; - - // expect_lint: avoid_global_state - static int globalMutable = 0; - - final int memberFinal = 1; - - int memberMutable = 0; - - void m() { - int localMutable = 0; - - final localFinal = 1; - - const localConst = 2; - } -} diff --git a/lint_test/avoid_non_null_assertion_test.dart b/lint_test/avoid_non_null_assertion_test.dart deleted file mode 100644 index 2a33a7df..00000000 --- a/lint_test/avoid_non_null_assertion_test.dart +++ /dev/null @@ -1,25 +0,0 @@ -// ignore_for_file: avoid_global_state, prefer_match_file_name -// ignore_for_file: member_ordering - -/// Check "bang" operator fail -/// -/// `avoid_non_null_assertion` -class AvoidNonNullAssertion { - AvoidNonNullAssertion? object; - int? number; - - void test() { - // expect_lint: avoid_non_null_assertion - number!; - - // expect_lint: avoid_non_null_assertion - object!.number!; - - // expect_lint: avoid_non_null_assertion - object!.test(); - - // No lint on maps - final map = {'key': 'value'}; - map['key']!; - } -} diff --git a/lint_test/avoid_returning_widget_test/analysis_options.yaml b/lint_test/avoid_returning_widget_test/analysis_options.yaml deleted file mode 100644 index 0953ea1e..00000000 --- a/lint_test/avoid_returning_widget_test/analysis_options.yaml +++ /dev/null @@ -1,11 +0,0 @@ -analyzer: - plugins: - - ../custom_lint - -custom_lint: - rules: - - avoid_returning_widgets: - exclude: - - class_name: ExcludeWidget - method_name: excludeWidgetMethod - - method_name: excludeMethod diff --git a/lint_test/avoid_returning_widget_test/avoid_returning_widget_test.dart b/lint_test/avoid_returning_widget_test/avoid_returning_widget_test.dart deleted file mode 100644 index ad4398ba..00000000 --- a/lint_test/avoid_returning_widget_test/avoid_returning_widget_test.dart +++ /dev/null @@ -1,96 +0,0 @@ -// ignore_for_file: unused_element, prefer_match_file_name -// ignore_for_file: member_ordering - -/// Check returning a widget fail -/// `avoid_returning_widgets` - -import 'package:flutter/material.dart'; - -// expect_lint: avoid_returning_widgets -Widget avoidReturningWidgets() => const SizedBox(); - -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 - Widget _test1() => const SizedBox(); - - // expect_lint: avoid_returning_widgets - Widget _test2() { - return const SizedBox( - child: SizedBox(), - ); - } - - // 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(); -} - -// no lint -SizedBox excludeMethod() => const SizedBox(); - -class ExcludeWidget extends StatelessWidget { - const ExcludeWidget({super.key}); - - @override - Widget build(BuildContext context) { - return const Placeholder(); - } - - // no lint - Widget excludeWidgetMethod() => const SizedBox(); - - // expect_lint: avoid_returning_widgets - Widget excludeWidgetMethod2() => const SizedBox(); -} - -class NotExcludeWidget extends StatelessWidget { - const NotExcludeWidget({super.key}); - - @override - Widget build(BuildContext context) { - return const Placeholder(); - } - - // expect_lint: avoid_returning_widgets - Widget excludeWidgetMethod() => const SizedBox(); -} diff --git a/lint_test/avoid_unnecessary_return_variable_test.dart b/lint_test/avoid_unnecessary_return_variable_test.dart deleted file mode 100644 index 1e068180..00000000 --- a/lint_test/avoid_unnecessary_return_variable_test.dart +++ /dev/null @@ -1,125 +0,0 @@ -// ignore_for_file: unused_local_variable, newline_before_return, no_empty_block - -/// Test the avoid_unnecessary_return_variable. -/// Good code, trivial case. -int returnVarTestGoodTrivial() { - return 1; -} - -/// Test the avoid_unnecessary_return_variable. -/// Returning mutable variable should not trigger the lint. -int returnVarTestReturnMutable() { - var a = 1; - a++; - - return a; -} - -int returnVarTestReturnParameter(int param) { - return param; -} - -/// Test the avoid_unnecessary_return_variable. -/// Caching mutable variable value. -/// Unpredictable: may be useful to cache value -/// before operation can change it. -int returnVarTestCachedMutable() { - var a = 1; - final result = a; - _doNothing(); - - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Caching mutable variable value, but return goes -/// right after declaration, which makes it bad. -int returnVarTestReturnFollowsDeclaration() { - var a = 1; - final result = a; - - //Some comment here - - //expect_lint: avoid_unnecessary_return_variable - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Caching another method result. -/// Unpredictable: may be useful to cache value -/// before operation can change it. -int returnVarTestCachedAnotherMethodResult() { - var a = 1; - final result = _testValueEval(); - _doNothing(); - - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Caching value of object's field. -/// Unpredictable: may be useful to cache value -/// before operation can change it. -int returnVarTestCachedObjectField() { - final obj = _TestClass(); - final result = obj.varField; - _doNothing(); - - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Good: variable is created not only for return -/// but is used in following expressions as well. -int returnVarTestUsedVariable() { - var a = 1; - final result = 2; - a += result; - - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Bad code, trivial example. -int returnVarTestBadTrivial() { - final result = 1; - - //expect_lint: avoid_unnecessary_return_variable - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Bad code: result expression is immutable, -/// so can be written in return statement directly. -int returnVarTestBadImmutableExpression() { - const constLocal = 1; - final finalLocal = 1; - final testObj = _TestClass(); - final result = constLocal + - finalLocal + - 1 + //const literal - _TestClass.constValue + - _TestClass.finalValue + - testObj.finalField; - _doNothing(); - - //expect_lint: avoid_unnecessary_return_variable - return result; -} - -int _testValueEval() { - return 1; -} - -/// This method is a placeholder for unpredictable behaviour -/// which can potentially change any mutable variables -void _doNothing() {} - -//ignore: prefer_match_file_name -class _TestClass { - static const constValue = 1; - static final finalValue = 1; - //ignore: member_ordering - final finalField = 1; - var varField = 1; -} diff --git a/lint_test/avoid_unnecessary_setstate_test.dart b/lint_test/avoid_unnecessary_setstate_test.dart deleted file mode 100644 index b308515b..00000000 --- a/lint_test/avoid_unnecessary_setstate_test.dart +++ /dev/null @@ -1,108 +0,0 @@ -// MIT License -// -// Copyright (c) 2020-2021 Dart Code Checker team -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -// ignore_for_file: member_ordering, prefer_match_file_name - -import 'package:flutter/material.dart'; - -/// Check unnecessary setstate fail -/// `avoid_unnecessary_setstate` -class MyWidget extends StatefulWidget { - @override - _MyWidgetState createState() => _MyWidgetState(); -} - -class _MyWidgetState extends State { - String _myString = ''; - final bool _condition = true; - - @override - void initState() { - super.initState(); - - methodWithoutSetState(); - - // expect_lint: avoid_unnecessary_setstate - setState(() { - _myString = "Hello"; - }); - - if (_condition) { - // expect_lint: avoid_unnecessary_setstate - setState(() { - _myString = "Hello"; - }); - } - - // expect_lint: avoid_unnecessary_setstate - myStateUpdateMethod(); - } - - @override - void didUpdateWidget(MyWidget oldWidget) { - super.didUpdateWidget(oldWidget); - // expect_lint: avoid_unnecessary_setstate - setState(() { - _myString = "Hello"; - }); - } - - void myStateUpdateMethod() { - setState(() { - _myString = "Hello"; - }); - } - - void methodWithoutSetState() { - _myString = 'hey'; - } - - @override - Widget build(BuildContext context) { - // expect_lint: avoid_unnecessary_setstate - setState(() { - _myString = "Hello"; - }); - - if (_condition) { - // expect_lint: avoid_unnecessary_setstate - setState(() { - _myString = "Hello"; - }); - } - - // expect_lint: avoid_unnecessary_setstate - myStateUpdateMethod(); - - return ElevatedButton( - onPressed: myStateUpdateMethod, - onHover: (_) => methodWithoutSetState(), - onLongPress: () { - setState(() { - _myString = 'data'; - }); - }, - child: Text(_myString), - ); - } -} diff --git a/lint_test/avoid_unnecessary_type_assertions_test.dart b/lint_test/avoid_unnecessary_type_assertions_test.dart deleted file mode 100644 index b3f2f3c6..00000000 --- a/lint_test/avoid_unnecessary_type_assertions_test.dart +++ /dev/null @@ -1,47 +0,0 @@ -// 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 - -/// Check the `avoid_unnecessary_type_assertions` rule - -void fun() { - final testList = [1.0, 2.0, 3.0]; - // expect_lint: avoid_unnecessary_type_assertions - final result = testList is List; - - // expect_lint: avoid_unnecessary_type_assertions - final negativeResult = testList is! List; - - // to check quick-fix => testList.length - // expect_lint: avoid_unnecessary_type_assertions - testList.whereType().length; - - final dynamicList = [1.0, 2.0]; - dynamicList.whereType(); - - // expect_lint: avoid_unnecessary_type_assertions - [1.0, 2.0].whereType(); - - final double d = 2.0; - // expect_lint: avoid_unnecessary_type_assertions - final casted = d is double; - - // expect_lint: avoid_unnecessary_type_assertions - final negativeCasted = d is! double; - - final double? nullableD = 2.0; - // 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 deleted file mode 100644 index 209a5615..00000000 --- a/lint_test/avoid_unrelated_type_assertions_test.dart +++ /dev/null @@ -1,49 +0,0 @@ -// 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 - -/// Check the `avoid_unrelated_type_assertions` rule -class Foo {} - -class Bar {} - -class ChildFoo extends Foo {} - -void fun() { - final testString = ''; - final testList = [1, 2, 3]; - final testMap = {'A': 'B'}; - final Foo foo = Foo(); - final childFoo = ChildFoo(); - - // expect_lint: avoid_unrelated_type_assertions - final result = testString is int; - - // expect_lint: avoid_unrelated_type_assertions - final result2 = testList is List; - - // expect_lint: avoid_unrelated_type_assertions - final result3 = foo is Bar; - - // expect_lint: avoid_unrelated_type_assertions - final result4 = childFoo is Bar; - - // 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/double_literal_format_test.dart b/lint_test/double_literal_format_test.dart deleted file mode 100644 index 195b01e8..00000000 --- a/lint_test/double_literal_format_test.dart +++ /dev/null @@ -1,38 +0,0 @@ -// ignore_for_file: avoid_global_state -// ignore_for_file: type_annotate_public_apis -// ignore_for_file: unused_local_variable - -/// Check the `double_literal_format` rule - -// expect_lint: double_literal_format -var badA = 05.23; -var goodA = 5.23; -var stringA = '05.23'; - -var intA = 0; - -// expect_lint: double_literal_format -double badB = -01.2; -double goodB = -1.2; - -// expect_lint: double_literal_format -double badC = -001.2; - -// expect_lint: double_literal_format -double badExpr = 5.23 + 05.23; - -double goodExpr = 5.23 + 5.23; - -class DoubleLiteralFormatTest { - // expect_lint: double_literal_format - var badA = .16e+5; - var goodA = 0.16e+5; - - void someMethod() { - // expect_lint: double_literal_format - const badA = -0.250; - const goodA = -0.25; - // expect_lint: double_literal_format - const badB = 0.160e+5; - } -} diff --git a/lint_test/function_lines_of_code_test/analysis_options.yaml b/lint_test/function_lines_of_code_test/analysis_options.yaml deleted file mode 100644 index 459a4d79..00000000 --- a/lint_test/function_lines_of_code_test/analysis_options.yaml +++ /dev/null @@ -1,14 +0,0 @@ -analyzer: - plugins: - - ../custom_lint - -custom_lint: - rules: - - function_lines_of_code: - max_lines: 5 - exclude: - - class_name: ClassWithLongMethods - method_name: longMethodExcluded - - method_name: longFunctionExcluded - - longFunctionExcludedByDeclarationName - - longMethodExcludedByDeclarationName 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 deleted file mode 100644 index 032f5cd6..00000000 --- a/lint_test/function_lines_of_code_test/function_lines_of_code_test.dart +++ /dev/null @@ -1,535 +0,0 @@ -// ignore_for_file: prefer_match_file_name - -class ClassWithLongMethods { - // expect_lint: function_lines_of_code - int longMethod() { - var i = 0; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - - return i; - } - - // Excluded by method_name - int longMethodExcluded() { - var i = 0; - i++; - i++; - i++; - i++; - - return i; - } - -// Excluded by declaration_name - int longMethodExcludedByDeclarationName() { - var i = 0; - i++; - i++; - i++; - i++; - - return i; - } - - int notLongMethod() { - var i = 0; - 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++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - - return i; -} - -// Excluded by method_name -int longFunctionExcluded() { - var i = 0; - i++; - i++; - i++; - i++; - - return i; -} - -// Excluded by declaration_name -int longFunctionExcludedByDeclarationName() { - var i = 0; - i++; - i++; - i++; - i++; - - return i; -} - -// expect_lint: function_lines_of_code -final longAnonymousFunction = () { - var i = 0; - i++; - i++; - i++; - i++; - - return i; -}; - -final notLongAnonymousFunction = () { - var i = 0; - i++; - i++; - i++; - - return i; -}; diff --git a/lint_test/newline_before_return_test.dart b/lint_test/newline_before_return_test.dart deleted file mode 100644 index 282057ad..00000000 --- a/lint_test/newline_before_return_test.dart +++ /dev/null @@ -1,37 +0,0 @@ -// ignore_for_file: unused_local_variable, prefer_match_file_name -// ignore_for_file: member_ordering -// ignore_for_file: avoid_unused_parameters - -/// Check the `newline_before_return` rule -class Foo { - int method() { - final a = 0; - // expect_lint: newline_before_return - return 1; - } - - void anotherMethod() { - final a = 1; - // expect_lint: newline_before_return - return; - } - - void bar(void Function()) { - return; - } -} - -void fun() { - final foo = Foo(); - foo.bar(() { - // This comment is ignored and line above is checked to be a newline - return; - }); - foo.bar(() { - final a = 1; - // expect_lint: newline_before_return - return; - }); - // expect_lint: newline_before_return - return; -} diff --git a/lint_test/no_equal_then_else_test.dart b/lint_test/no_equal_then_else_test.dart deleted file mode 100644 index c9caa40c..00000000 --- a/lint_test/no_equal_then_else_test.dart +++ /dev/null @@ -1,30 +0,0 @@ -// ignore_for_file: unused_local_variable -// ignore_for_file: cyclomatic_complexity -// ignore_for_file: no_magic_number -// ignore_for_file: prefer_conditional_expressions - -/// Check the `no_equal_then_else` rule -void fun() { - final _valueA = 1; - final _valueB = 2; - - int _result = 0; - - // expect_lint: no_equal_then_else - if (_valueA == 1) { - _result = _valueA; - } else { - _result = _valueA; - } - - if (_valueA == 1) { - _result = _valueA; - } else { - _result = _valueB; - } - - // expect_lint: no_equal_then_else - _result = _valueA == 2 ? _valueA : _valueA; - - _result = _valueA == 2 ? _valueA : _valueB; -} diff --git a/lint_test/prefer_early_return_test.dart b/lint_test/prefer_early_return_test.dart deleted file mode 100644 index d9108186..00000000 --- a/lint_test/prefer_early_return_test.dart +++ /dev/null @@ -1,299 +0,0 @@ -// ignore_for_file: dead_code, cyclomatic_complexity, no_equal_then_else, prefer_match_file_name - -// ignore: no_empty_block -void _doSomething() {} - -void oneIf() { - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} - -int oneIfWithReturnValue() { - //no lint - if (true) { - _doSomething(); - } - - return 1; -} - -void oneIfWithReturn() { - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } - - return; -} - -void nestedIf2() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - _doSomething(); - } - } -} - -int nestedIf2WithReturnValue() { - //no lint - if (true) { - if (true) { - _doSomething(); - } - } - - return 1; -} - -void nestedIf3() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - if (true) { - _doSomething(); - } - } - } -} - -void oneNestedIf2WithReturn() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - if (true) { - _doSomething(); - } - } - } - - return; -} - -void oneIfElse() { - //no lint - if (true) { - _doSomething(); - } else { - _doSomething(); - } -} - -void oneIfElseReturn() { - //no lint - if (true) { - _doSomething(); - } else { - return; - } -} - -void twoIfElse() { - //no lint - if (true) { - if (true) { - _doSomething(); - } - } else { - _doSomething(); - } -} - -void twoIfElseInner() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - _doSomething(); - } else { - _doSomething(); - } - } -} - -void threeIf() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - if (true) { - _doSomething(); - } - } - } -} - -void threeIfElse1() { - //no lint - if (true) { - if (true) { - if (true) { - _doSomething(); - } - } - } else { - _doSomething(); - } -} - -void threeIfElse2() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - if (true) { - _doSomething(); - } - } else { - _doSomething(); - } - } -} - -void threeIfElse3() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - if (true) { - _doSomething(); - } else { - _doSomething(); - } - } - } -} - -void twoSeqentialIf() { - if (false) return; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} - -void twoSeqentialIfReturn() { - //no lint - if (false) return; - if (true) return; - - return; -} - -void twoSeqentialIfReturn2() { - //no lint - if (false) return; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } - - return; -} - -void twoSeqentialIfSomething() { - //no lint - if (false) return; - //no lint - if (true) { - _doSomething(); - } - - _doSomething(); -} - -void twoSeqentialIfSomething2() { - //no lint - if (true) { - _doSomething(); - } - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} - -void threeSeqentialIfReturn() { - //no lint - if (false) return; - if (true) return; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } - - return; -} - -void threeSeqentialIfReturn2() { - //no lint - if (false) return; - //no lint - if (true) { - _doSomething(); - } - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} - -void oneIfWithThrowWithReturn() { - //no lint - if (true) { - throw ''; - } - - return; -} - -void oneIfElseWithThrowReturn() { - //no lint - if (true) { - _doSomething(); - } else { - throw ''; - } -} - -void twoSeqentialIfWithThrow() { - if (false) throw ''; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} - -void twoSeqentialIfWithThrowReturn2() { - //no lint - if (false) throw ''; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } - - return; -} - -void threeSeqentialIfWithThrowReturn() { - //no lint - if (false) throw ''; - if (true) throw ''; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } - - return; -} - -void threeSeqentialIfWithThrowReturn2() { - //no lint - if (false) throw ''; - //no lint - if (true) { - _doSomething(); - } - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} diff --git a/lint_test/proper_super_calls_test.dart b/lint_test/proper_super_calls_test.dart deleted file mode 100644 index fc6e4773..00000000 --- a/lint_test/proper_super_calls_test.dart +++ /dev/null @@ -1,82 +0,0 @@ -// ignore_for_file: prefer_match_file_name, no_empty_block - -class Widget {} - -abstract class State { - build(); - dispose() {} - initState() {} -} - -abstract class StatefulWidget extends Widget { - State createState(); - - StatefulWidget(); -} - -/// Check "check super" keyword fail -/// -/// `proper_super_calls` -class ProperSuperCallsTest1 extends StatefulWidget { - @override - State createState() => _ProperSuperCallsTest1State(); - - ProperSuperCallsTest1(); -} - -class _ProperSuperCallsTest1State extends State { - @override - Widget build() { - return Widget(); - } - - // expect_lint: proper_super_calls - @override - void initState() { - print(''); - super.initState(); - } - - // expect_lint: proper_super_calls - @override - void dispose() { - super.dispose(); - print(''); - } -} - -class ProperSuperCallsTest2 extends StatefulWidget { - @override - State createState() => _ProperSuperCallsTest2State(); - - ProperSuperCallsTest2(); -} - -class _ProperSuperCallsTest2State extends State { - @override - Widget build() { - return Widget(); - } - - @override - void initState() { - super.initState(); - print(''); - } - - @override - void dispose() { - print(''); - super.dispose(); - } -} - -class MyClass { - dispose() {} - initState() {} -} - -abstract interface class Disposable { - /// Abstract methods should be omitted by `proper_super_calls` - void dispose(); -} diff --git a/lint_test/pubspec.yaml b/lint_test/pubspec.yaml index cb37cd0b..10e64475 100644 --- a/lint_test/pubspec.yaml +++ b/lint_test/pubspec.yaml @@ -1,9 +1,9 @@ name: solid_lints_test -description: A starting point for Dart libraries or applications. +description: Test project for solid_lints rules publish_to: none environment: - sdk: '>=3.0.0 <4.0.0' + sdk: ">=3.5.0 <4.0.0" dependencies: flutter: diff --git a/pubspec.yaml b/pubspec.yaml index 80a1415e..b1a226bb 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -8,19 +8,22 @@ documentation: https://solid-software.github.io/solid_lints/docs/intro topics: [lints, linter, lint, analysis, analyzer] environment: - sdk: ">=3.5.0 <4.0.0" + sdk: ">=3.9.0 <4.0.0" dependencies: - analyzer: ^8.4.0 + # Needed until required types for fixes are exported by analyzer_server_plugin + # More details: https://github.com/dart-lang/sdk/issues/61821 + analyzer_plugin: ^0.14.2 + analyzer: ^10.0.1 collection: ^1.19.0 - custom_lint_builder: ^0.8.1 + analysis_server_plugin: ^0.3.3 glob: ^2.1.3 path: ^1.9.1 yaml: ^3.1.3 # These packages are required for pana analysis to run correctly - flutter: - sdk: flutter test: ^1.25.14 dev_dependencies: args: ^2.6.0 + analyzer_testing: ^0.1.9 + test_reflective_loader: ^0.3.0 diff --git a/test/lints/auto_test_lint_offsets.dart b/test/lints/auto_test_lint_offsets.dart new file mode 100644 index 00000000..a9b4b298 --- /dev/null +++ b/test/lints/auto_test_lint_offsets.dart @@ -0,0 +1,55 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:analyzer_testing/src/analysis_rule/pub_package_resolution.dart'; +import 'package:collection/collection.dart'; + +mixin AutoTestLintOffsets on AnalysisRuleTest { + int _nextPlaceholderId = 0; + final Map _placeholderToCode = {}; + + Future assertAutoDiagnostics(String source) async { + try { + final placeholders = _placeholderToCode.entries + .map( + (entry) => ( + placeholder: entry.key, + code: entry.value, + index: source.indexOf(entry.key), + ), + ) + .sorted((a, b) => a.index.compareTo(b.index)); + + final expectedDiagnostics = []; + int replacedPlaceholdersDelta = 0; + + for (final match in placeholders) { + if (match.index == -1) { + throw StateError( + 'Expected lint placeholder "${match.placeholder}" was not found in source.', + ); + } + + expectedDiagnostics.add( + lint(match.index + replacedPlaceholdersDelta, match.code.length), + ); + replacedPlaceholdersDelta += + match.code.length - match.placeholder.length; + } + + final resolvedSource = _placeholderToCode.entries.fold( + source, + (resolved, entry) => resolved.replaceFirst(entry.key, entry.value), + ); + + await assertDiagnostics(resolvedSource, expectedDiagnostics); + } finally { + _nextPlaceholderId = 0; + _placeholderToCode.clear(); + } + } + + String expectLint(String code) { + final placeholder = '__AUTO_TEST_LINT_${_nextPlaceholderId++}__'; + _placeholderToCode[placeholder] = code; + return placeholder; + } +} diff --git a/test/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule_test.dart b/test/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule_test.dart new file mode 100644 index 00000000..74c6554b --- /dev/null +++ b/test/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule_test.dart @@ -0,0 +1,186 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidDebugPrintInReleaseRuleTest); + }); +} + +@reflectiveTest +class AvoidDebugPrintInReleaseRuleTest extends AnalysisRuleTest { + @override + void setUp() { + final flutter = newPackage('flutter'); + + flutter.addFile('lib/foundation.dart', r''' +const bool kReleaseMode = false; +const bool kDebugMode = true; +void debugPrint(String? message) {} +'''); + + flutter.addFile('lib/material.dart', r''' +export 'package:flutter/foundation.dart'; +'''); + + flutter.addFile('lib/cupertino.dart', r''' +export 'package:flutter/foundation.dart'; +'''); + + rule = AvoidDebugPrintInReleaseRule(); + + super.setUp(); + } + + @override + String get analysisRule => AvoidDebugPrintInReleaseRule.lintName; + + void test_reports_debug_print_with_package_import() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + debugPrint('This should be flagged'); +} +''', + [lint(59, 10)], + ); + } + + void test_reports_aliased_debug_print_from_package() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart' as f; + +void test() { + f.debugPrint('This should be flagged'); +} +''', + [lint(66, 10)], + ); + } + + void test_reports_debug_print_as_callback() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + ['a'].forEach(debugPrint); +} +''', + [lint(73, 10)], + ); + } + + void test_reports_inside_kReleaseMode_guard() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + if (kReleaseMode) { + debugPrint('This should be flagged'); + } +} +''', + [lint(83, 10)], + ); + } + + void test_reports_inside_not_kDebugMode_guard() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + if (!kDebugMode) { + debugPrint('Should still be flagged'); + } +} +''', + [lint(82, 10)], + ); + } + + void test_does_not_report_inside_not_kReleaseMode() async { + await assertNoDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + if (!kReleaseMode) { + debugPrint('Safe'); + } +} +''', + ); + } + + void test_does_not_report_inside_kDebugMode() async { + await assertNoDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + if (kDebugMode) { + debugPrint('Safe'); + } +} +''', + ); + } + + void test_no_report_when_debugPrint_is_not_from_foundation() async { + await assertNoDiagnostics( + r''' +void debugPrint(String message) {} + +void test() { + debugPrint('Not a flutter call'); +} +''', + ); + } + + void test_reports_when_imported_via_material() async { + await assertDiagnostics( + r''' +import 'package:flutter/material.dart'; + +void test() { + debugPrint('Flagged via material'); +} +''', + [lint(57, 10)], + ); + } + + void test_reports_when_imported_via_cupertino() async { + await assertDiagnostics( + r''' +import 'package:flutter/cupertino.dart'; + +void test() { + debugPrint('Flagged via cupertino'); +} +''', + [lint(58, 10)], + ); + } + + void test_reports_debug_print_call_method() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + debugPrint.call('This should be flagged'); +} +''', + [lint(59, 10)], + ); + } +} diff --git a/test/lints/avoid_global_state/avoid_global_state_rule_test.dart b/test/lints/avoid_global_state/avoid_global_state_rule_test.dart new file mode 100644 index 00000000..29616804 --- /dev/null +++ b/test/lints/avoid_global_state/avoid_global_state_rule_test.dart @@ -0,0 +1,76 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/avoid_global_state/avoid_global_state_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidGlobalStateRuleTest); + }); +} + +@reflectiveTest +class AvoidGlobalStateRuleTest extends AnalysisRuleTest { + @override + void setUp() { + rule = AvoidGlobalStateRule(); + super.setUp(); + } + + void test_reports_mutable_top_level_variable() async { + await assertDiagnostics( + r''' +var globalMutable = 0; +''', + [lint(4, 17)], + ); + } + + void test_reports_mutable_static_field() async { + await assertDiagnostics( + r''' +class Test { + static int staticMutable = 0; +} +''', + [lint(26, 17)], + ); + } + + void test_does_not_report_global_immutable_variables() async { + await assertNoDiagnostics(r''' +final globalFinal = 1; +const globalConst = 1; +'''); + } + + void test_does_not_report_global_private_variables() async { + await assertNoDiagnostics(r''' +var _privateTopLevel = 0; +'''); + } + + void test_does_not_report_class_level_immutable_variables() async { + await assertNoDiagnostics(r''' +class Test { + static final int staticFinal = 1; + static const int staticConst = 2; +} +'''); + } + + void test_does_not_report_class_level_private_variables() async { + await assertNoDiagnostics(r''' +class Test { + static int _staticPrivate = 0; +} +'''); + } + + void test_does_not_report_local_variables() async { + await assertNoDiagnostics(r''' +void m() { + int localMutable = 0; +} +'''); + } +} diff --git a/test/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule_test.dart b/test/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule_test.dart new file mode 100644 index 00000000..8d054cda --- /dev/null +++ b/test/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule_test.dart @@ -0,0 +1,59 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidNonNullAssertionRuleTest); + }); +} + +@reflectiveTest +class AvoidNonNullAssertionRuleTest extends AnalysisRuleTest { + @override + void setUp() { + rule = AvoidNonNullAssertionRule(); + super.setUp(); + } + + void test_reports_non_null_assertion_on_nullable_value() async { + await assertDiagnostics( + r''' +void m(int? number) { + final value = number!; +} +''', + [lint(38, 7)], + ); + } + + void test_reports_non_null_assertion_on_method_call() async { + await assertDiagnostics( + r''' +void m(Object? object) { + object!.toString(); +} +''', + [lint(27, 7)], + ); + } + + void test_does_not_report_map_access() async { + await assertNoDiagnostics(r''' +void m() { + final map = {'key': 'value'}; + map['key']!; +} +'''); + } + + void test_does_not_report_safe_null_check() async { + await assertNoDiagnostics(r''' +void m(int? number) { + if (number != null) { + final value = number; + } +} +'''); + } +} diff --git a/test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart b/test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart new file mode 100644 index 00000000..d2e42b16 --- /dev/null +++ b/test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart @@ -0,0 +1,175 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidUnnecessaryReturnVariableTest); + }); +} + +@reflectiveTest +class AvoidUnnecessaryReturnVariableTest extends AnalysisRuleTest { + @override + void setUp() { + rule = AvoidUnnecessaryReturnVariableRule(); + super.setUp(); + } + + @override + String get analysisRule => rule.name; + + void test_does_not_report_if_return_good_trivial() async { + await assertNoDiagnostics(r''' +int returnVarTestTrivial() { + return 1; +} + '''); + } + + void test_does_not_report_if_return_is_mutable() async { + await assertNoDiagnostics(r''' +int returnVarTestMutable() { + var a = 1; + a++; + + return a; +} + '''); + } + + void test_does_not_report_if_returns_parameter() async { + await assertNoDiagnostics(r''' +int returnVarTestReturnParameter(int param) { + return param; +} + '''); + } + + void test_does_not_report_if_return_is_cached_mutable() async { + await assertNoDiagnostics(r''' +int returnVarTestCachedMutable() { + var a = 1; + final result = a; + _doNothing(); + + return result; +} + +void _doNothing() {} + '''); + } + + void test_reports_if_return_follows_declaration() async { + await assertDiagnostics( + r''' +int returnVarTestReturnFollowsDeclaration() { + var a = 1; + final result = a; + + //Some comment here + + return result; +} + ''', + [lint(105, 14)], + ); + } + + void test_does_not_report_if_return_is_cached_another_method_result() async { + await assertNoDiagnostics(r''' +int returnVarTestCachedAnotherMethodResult() { + var a = 1; + final result = _testValueEval(); + _doNothing(); + + return result; +} + +int _testValueEval() { + return 1; +} + +void _doNothing() {} +'''); + } + + void test_does_not_report_if_return_is_cached_object_field() async { + await assertNoDiagnostics(r''' +int returnVarTestCachedObjectField() { + final obj = _TestClass(); + final result = obj.varField; + _doNothing(); + + return result; +} + +class _TestClass { + static const constValue = 1; + static final finalValue = 1; + //ignore: member_ordering + final finalField = 1; + var varField = 1; +} + +void _doNothing() {} +'''); + } + + void test_does_not_report_if_return_used_variable() async { + await assertNoDiagnostics(r''' +int returnVarTestUsedVariable() { + var a = 1; + final result = 2; + a += result; + + return result; +} +'''); + } + + void test_reports_if_return_is_bad_trivial() async { + await assertDiagnostics( + r''' +int returnVarTestBadTrivial() { + final result = 1; + + return result; +} +''', + [lint(55, 14)], + ); + } + + void test_reports_if_return_is_bad_immutable_expression() async { + await assertDiagnostics( + r''' +int returnVarTestBadImmutableExpression() { + const constLocal = 1; + final finalLocal = 1; + final testObj = _TestClass(); + final result = constLocal + + finalLocal + + 1 + //const literal + _TestClass.constValue + + _TestClass.finalValue + + testObj.finalField; + _doNothing(); + + return result; +} + +class _TestClass { + static const constValue = 1; + static final finalValue = 1; + //ignore: member_ordering + final finalField = 1; + var varField = 1; +} + +void _doNothing() {} +''', + [lint(304, 14)], + ); + } +} diff --git a/test/lints/avoid_unnecessary_set_state/avoid_unnecessary_set_state_rule_test.dart b/test/lints/avoid_unnecessary_set_state/avoid_unnecessary_set_state_rule_test.dart new file mode 100644 index 00000000..0e160ae0 --- /dev/null +++ b/test/lints/avoid_unnecessary_set_state/avoid_unnecessary_set_state_rule_test.dart @@ -0,0 +1,330 @@ +// MIT License +// +// Copyright (c) 2020-2021 Dart Code Checker team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidUnnecessarySetStateRuleTest); + }); +} + +@reflectiveTest +class AvoidUnnecessarySetStateRuleTest extends AnalysisRuleTest { + @override + void setUp() { + final flutter = newPackage('flutter'); + + flutter.addFile( + 'lib/src/widgets/framework.dart', + r''' +abstract class Widget {} + +abstract class StatefulWidget extends Widget {} + +class BuildContext {} + +abstract class State { + void initState() {} + void didUpdateWidget(T oldWidget) {} + void setState(void Function() fn) {} + + Widget build(BuildContext context); +} + +class Text extends Widget { + final Object? data; + Text(this.data); +} + +class ElevatedButton extends Widget { + final Function()? onPressed; + final Function()? onLongPress; + final Widget? child; + + ElevatedButton({this.onPressed, this.onLongPress, this.child}); +} +''', + ); + + rule = AvoidUnnecessarySetStateRule(); + super.setUp(); + } + + @override + String get analysisRule => rule.name; + + void test_reports_set_state_in_init_state() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class _MyWidgetState extends State { + String _myString = ''; + + @override + void initState() { + super.initState(); + + setState(() { + _myString = "Hello"; + }); + } + + @override + Widget build(BuildContext context) { + return ElevatedButton( + child: Text(_myString), + ); + } +} + ''', + [lint(194, 47)], + ); + } + + void test_reports_set_state_in_init_state_with_condition() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class _MyWidgetState extends State { + String _myString = ''; + final bool _condition = true; + + @override + void initState() { + super.initState(); + + if (_condition) { + setState(() { + _myString = "Hello"; + }); + } + } + + @override + Widget build(BuildContext context) { + return ElevatedButton( + child: Text(_myString), + ); + } +} + ''', + [lint(250, 51)], + ); + } + + void test_reports_set_state_in_init_state_through_method() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class _MyWidgetState extends State { + String _myString = ''; + final bool _condition = true; + + @override + void initState() { + super.initState(); + + myStateUpdateMethod(); + } + + void myStateUpdateMethod() { + setState(() { + _myString = "Hello"; + }); + } + + @override + Widget build(BuildContext context) { + return ElevatedButton( + child: Text(_myString), + ); + } +} + ''', + [lint(226, 21)], + ); + } + + void test_reports_set_state_in_did_update_widget() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class _MyWidgetState extends State { + String _myString = ''; + + @override + void didUpdateWidget(StatefulWidget oldWidget) { + super.didUpdateWidget(oldWidget); + setState(() { + _myString = "Hello"; + }); + } + + @override + Widget build(BuildContext context) { + return ElevatedButton( + child: Text(_myString), + ); + } +} + ''', + [lint(238, 47)], + ); + } + + void test_reports_set_state_in_build_method() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class _MyWidgetState extends State { + String _myString = ''; + + @override + Widget build(BuildContext context) { + setState(() { + _myString = "Hello"; + }); + + return ElevatedButton( + child: Text(_myString), + ); + } +} + ''', + [lint(188, 47)], + ); + } + + void test_reports_set_state_in_build_method_with_condition() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class _MyWidgetState extends State { + String _myString = ''; + final bool _condition = true; + + @override + Widget build(BuildContext context) { + if (_condition) { + setState(() { + _myString = "Hello"; + }); + } + + return ElevatedButton( + child: Text(_myString), + ); + } +} + ''', + [lint(244, 51)], + ); + } + + void test_reports_set_state_in_build_method_through_method() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class _MyWidgetState extends State { + String _myString = ''; + + void myStateUpdateMethod() { + setState(() { + _myString = "Hello"; + }); + } + + @override + Widget build(BuildContext context) { + myStateUpdateMethod(); + + return ElevatedButton( + child: Text(_myString), + ); + } +} + ''', + [lint(277, 21)], + ); + } + + void test_does_not_report_set_state_in_button_on_pressed() async { + await assertNoDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class _MyWidgetState extends State { + String _myString = ''; + + void myStateUpdateMethod() { + setState(() { + _myString = "Hello"; + }); + } + + @override + Widget build(BuildContext context) { + return ElevatedButton( + onPressed: myStateUpdateMethod, + child: Text(_myString), + ); + } +} + ''', + ); + } + + void test_does_not_report_set_state_in_button_on_long_press() async { + await assertNoDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class _MyWidgetState extends State { + String _myString = ''; + + @override + Widget build(BuildContext context) { + return ElevatedButton( + onLongPress: () { + setState(() { + _myString = 'data'; + }); + }, + child: Text(_myString), + ); + } +} + ''', + ); + } +} diff --git a/test/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule_test.dart b/test/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule_test.dart new file mode 100644 index 00000000..602e60ba --- /dev/null +++ b/test/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule_test.dart @@ -0,0 +1,199 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../auto_test_lint_offsets.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidUnnecessaryTypeAssertionsRuleTest); + }); +} + +@reflectiveTest +class AvoidUnnecessaryTypeAssertionsRuleTest extends AnalysisRuleTest + with AutoTestLintOffsets { + @override + void setUp() { + rule = AvoidUnnecessaryTypeAssertionsRule(); + super.setUp(); + } + + @override + String get analysisRule => AvoidUnnecessaryTypeAssertionsRule.lintName; + + Future + test_does_not_report_if_is_expression_checks_nullable_source() async { + await assertNoDiagnostics(r''' +void fun() { + final double? nullableD = 2.0; + final castedD = nullableD is double; +} +'''); + } + + Future + test_does_not_report_if_is_not_expression_checks_unrelated_subtype() async { + await assertNoDiagnostics(r''' +class _A {} + +class _B extends _A {} + +class _C extends _A {} + +void fun() { + final _A a = _B(); + if (a is! _C) return; +} +'''); + } + + Future + test_does_not_report_if_is_expression_has_different_generic_type_argument() async { + await assertNoDiagnostics(r''' +void fun() { + final nums = [1, 2, 3]; + final result = nums is List; +} +'''); + } + + Future + test_does_not_report_if_where_type_filters_dynamic_iterable() async { + await assertNoDiagnostics(r''' +void fun() { + final dynamicList = [1.0, 2.0]; + dynamicList.whereType(); +} +'''); + } + + Future + test_does_not_report_if_where_type_has_different_generic_type_argument() async { + await assertNoDiagnostics(r''' +void fun() { + final nums = [1, 2, 3]; + nums.whereType(); +} +'''); + } + + Future test_does_not_report_if_where_type_omits_type_argument() async { + await assertNoDiagnostics(r''' +void fun() { + final values = [1.0, 2.0, 3.0]; + values.whereType(); +} +'''); + } + + Future test_reports_if_is_expression_checks_exact_generic_type() async { + await assertAutoDiagnostics(''' +// ignore_for_file: unnecessary_type_check + +void fun() { + final testList = [1.0, 2.0, 3.0]; + final result = ${expectLint('testList is List')}; +} +'''); + } + + Future + test_reports_if_is_not_expression_checks_exact_generic_type() async { + await assertAutoDiagnostics(''' +// ignore_for_file: unnecessary_type_check + +void fun() { + final testList = [1.0, 2.0, 3.0]; + final result = ${expectLint('testList is! List')}; +} +'''); + } + + Future test_reports_if_is_expression_checks_exact_scalar_type() async { + await assertAutoDiagnostics(''' +// ignore_for_file: unnecessary_type_check + +void fun() { + final double d = 2.0; + final casted = ${expectLint('d is double')}; +} +'''); + } + + Future + test_reports_if_is_not_expression_checks_exact_scalar_type() async { + await assertAutoDiagnostics(''' +// ignore_for_file: unnecessary_type_check + +void fun() { + final double d = 2.0; + final negativeCasted = ${expectLint('d is! double')}; +} +'''); + } + + Future test_reports_if_is_expression_checks_nullable_target() async { + await assertAutoDiagnostics(''' +// ignore_for_file: unnecessary_type_check + +void fun() { + final double d = 2.0; + final casted = ${expectLint('d is double?')}; +} +'''); + } + + Future test_reports_if_is_expression_checks_supertype() async { + await assertAutoDiagnostics(''' +// ignore_for_file: unnecessary_type_check + +void fun() { + final ints = [1, 2, 3]; + final result = ${expectLint('ints is Iterable')}; +} +'''); + } + + Future test_reports_if_where_type_filters_exact_type() async { + await assertAutoDiagnostics(''' +void fun() { + final testList = [1.0, 2.0, 3.0]; + ${expectLint('testList.whereType()')}.length; +} +'''); + } + + Future test_reports_if_where_type_filters_nullable_type() async { + await assertAutoDiagnostics(''' +void fun() { + ${expectLint('[1.0, 2.0].whereType()')}; +} +'''); + } + + Future test_reports_on_custom_types_with_default_generics() async { + await assertAutoDiagnostics(''' +// ignore_for_file: unnecessary_type_check + +abstract class MyIterable implements Iterable {} + +void test(MyIterable iterable) { + ${expectLint('iterable.whereType()')}; +} +'''); + } + + Future test_does_not_report_custom_types_when_using_subtype() async { + await assertNoDiagnostics(r''' +abstract class _A {} +abstract class _B extends _A {} + +abstract class AIterable implements Iterable<_A> {} + +void test(AIterable iterable) { + iterable.whereType<_B>(); +} +'''); + } +} diff --git a/test/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule_test.dart b/test/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule_test.dart new file mode 100644 index 00000000..a2260724 --- /dev/null +++ b/test/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule_test.dart @@ -0,0 +1,148 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidUnrelatedTypeAssertionsRuleTest); + }); +} + +@reflectiveTest +class AvoidUnrelatedTypeAssertionsRuleTest extends AnalysisRuleTest { + @override + void setUp() { + rule = AvoidUnrelatedTypeAssertionsRule(); + super.setUp(); + } + + @override + String get analysisRule => rule.name; + + void test_reports_unrelated_string_is_int() async { + await assertDiagnostics( + r''' +void fun() { + final testString = ''; + + final result = testString is int; +} +''', + [ + lint(56, 17, messageContainsAll: ['false']), + ], + ); + } + + void test_reports_unrelated_int_list_is_string_list() async { + await assertDiagnostics( + r''' +void fun() { + final testList = [1, 2, 3]; + + final result = testList is List; +} +''', + [ + lint(61, 24, messageContainsAll: ['false']), + ], + ); + } + + void test_reports_unrelated_string_map_is_double_map() async { + await assertDiagnostics( + r''' +void fun() { + final testMap = {'A': 'B'}; + + final result = testMap['A'] is double; +} +''', + [ + lint(61, 22, messageContainsAll: ['false']), + ], + ); + } + + void test_reports_unrelated_class_is_another_class() async { + await assertDiagnostics( + r''' +class Foo {} + +class Bar {} + +void fun() { + final Foo foo = Foo(); + + final result = foo is Bar; +} +''', + [ + lint(84, 10, messageContainsAll: ['false']), + ], + ); + } + + void test_reports_unrelated_child_class_is_another_class() async { + await assertDiagnostics( + r''' +class Foo {} + +class Bar {} + +class ChildFoo extends Foo {} + +void fun() { + final childFoo = ChildFoo(); + + final result = childFoo is Bar; +} +''', + [ + lint(121, 15, messageContainsAll: ['false']), + ], + ); + } + + void test_reports_unrelated_is_condition() async { + await assertDiagnostics( + r''' +class _A {} + +class _B extends _A {} + +class _C {} + +void lint() { + final _A a = _B(); + + if (a is _C) return; +} +''', + [ + lint(92, 7, messageContainsAll: ['false']), + ], + ); + } + + void test_reports_unrelated_is_not_condition() async { + await assertDiagnostics( + r''' +class _A {} + +class _B extends _A {} + +class _C {} + +void lint() { + final _A a = _B(); + + if (a is! _C) return; +} +''', + [ + lint(92, 8, messageContainsAll: ['true']), + ], + ); + } +} diff --git a/test/lints/double_literal_format/double_literal_format_rule_test.dart b/test/lints/double_literal_format/double_literal_format_rule_test.dart new file mode 100644 index 00000000..d3c8c2a7 --- /dev/null +++ b/test/lints/double_literal_format/double_literal_format_rule_test.dart @@ -0,0 +1,139 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(DoubleLiteralFormatRuleTest); + }); +} + +@reflectiveTest +class DoubleLiteralFormatRuleTest extends AnalysisRuleTest { + @override + void setUp() { + rule = DoubleLiteralFormatRule(); + super.setUp(); + } + + Future test_reports_on_leading_zeros() async { + await assertDiagnostics( + r''' +var badA = 05.23; +double badB = -01.2; +double badC = -001.2; +double badExpr = 5.23 + 05.23; + +class Test { + var badA = 05.23; + double badB = -01.2; + double badC = -001.2; + double badExpr = 5.23 + 05.23; +} +''', + [ + lint(11, 5), + lint(33, 4), + lint(54, 5), + lint(85, 5), + + lint(119, 5), + lint(143, 4), + lint(166, 5), + lint(199, 5), + ], + ); + } + + Future test_reports_on_trailing_zeros() async { + await assertDiagnostics( + r''' +class Test { + var badA = 5.230; + final badB = -1.20; + double get badC => -1.200; + double badExpr = 5.23 + 5.230; + var badD = -0.400e-5; + + void someMethod() { + var badA = 5.230; + double badB = -1.20; + double badC = -1.200; + double badExpr = 5.23 + 5.230; + var badD = -0.400E-5; + } +} +''', + [ + lint(26, 5), + lint(49, 4), + lint(77, 5), + lint(110, 5), + lint(131, 8), + + lint(179, 5), + lint(205, 4), + lint(230, 5), + lint(265, 5), + lint(288, 8), + ], + ); + } + + Future test_reports_on_leading_decimal_point() async { + await assertDiagnostics( + r''' +var badA = .23; +double badB = -.2; +double badExpr = 5.23 + .23; +var badD = .4e-5; + +class Test { + var badA = .23; + double badB = -.2; + double get badExpr => 5.23 + .23; + double get badD => -.4E-5; +} +''', + [ + lint(11, 3), + lint(31, 2), + lint(59, 3), + lint(75, 5), + + lint(109, 3), + lint(131, 2), + lint(166, 3), + lint(193, 5), + ], + ); + } + + void test_does_not_report_on_non_double_literals() async { + await assertNoDiagnostics(r''' +var badA = '05.23'; +var stringA = '5.23'; +final badB = '.04'; +var intA = 0; +'''); + } + + Future test_does_not_report_on_good_literals() async { + await assertNoDiagnostics(r''' +var goodA = 5.23; +double goodB = -1.2; + +double goodExpr = 5.23 + 5.23; +class DoubleLiteralFormatTest { + var goodA = 0.16e+5; + var goodB = 0.16E+5; + double goodC = -0.4E-5; + double goodE = 5.23 + 0.4e-5; + + void someMethod() { + const goodA = -0.25; + } +} +'''); + } +} diff --git a/test/lints/newline_before_return/newline_before_return_rule_test.dart b/test/lints/newline_before_return/newline_before_return_rule_test.dart new file mode 100644 index 00000000..db805301 --- /dev/null +++ b/test/lints/newline_before_return/newline_before_return_rule_test.dart @@ -0,0 +1,171 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/newline_before_return/newline_before_return_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(NewlineBeforeReturnRuleTest); + }); +} + +@reflectiveTest +class NewlineBeforeReturnRuleTest extends AnalysisRuleTest { + @override + void setUp() { + rule = NewlineBeforeReturnRule(); + super.setUp(); + } + + @override + String get analysisRule => rule.name; + + void test_reports_no_newline_before_return_value() async { + await assertDiagnostics( + r''' +int method() { + final a = 0; + return 1; +} + ''', + [lint(32, 9)], + ); + } + + void test_reports_no_newline_before_return() async { + await assertDiagnostics( + r''' +void method() { + final a = 0; + return; +} + ''', + [lint(33, 7)], + ); + } + + void test_reports_no_newline_before_return_with_comment() async { + await assertDiagnostics( + r''' +void method() { + final a = 0; + // Comment + return; +} + ''', + [lint(46, 7)], + ); + } + + void test_reports_no_newline_before_return_with_multiple_comments() async { + await assertDiagnostics( + r''' +void method() { + final a = 0; + // Comment 1 + // Comment 2 + return; +} + ''', + [lint(63, 7)], + ); + } + + void test_does_not_report_newline_before_return_with_comment() async { + await assertNoDiagnostics(r''' +void method() { + final a = 0; + + // Comment + return; +} + '''); + } + + void test_does_not_report_no_newline_before_single_statement_return() async { + await assertNoDiagnostics(r''' +void method() { + return; +} + '''); + } + + void test_does_not_report_newline_before_return() async { + await assertNoDiagnostics(r''' +void method() { + final a = 0; + + return; +} + '''); + } + + void + test_does_not_report_no_newline_before_single_statement_return_value() async { + await assertNoDiagnostics(r''' +int method() { + return 1; +} + '''); + } + + void + test_does_not_report_no_newline_before_single_statement_nested_return() async { + await assertNoDiagnostics(r''' +class Foo{ + void bar(void Function()) { + return; + } +} + +void fun() { + final foo = Foo(); + foo.bar(() { + return; + }); +} + '''); + } + + void test_reports_no_newline_before_return_nested() async { + await assertDiagnostics( + r''' +class Foo{ + void bar(void Function()) { + return; + } +} + +void fun() { + final foo = Foo(); + foo.bar(() { + final a = 1; + return; + }); +} + ''', + [lint(130, 7)], + ); + } + + void test_reports_no_newline_before_two_return_nested() async { + await assertDiagnostics( + r''' +class Foo{ + void bar(void Function()) { + return; + } +} + +void fun() { + final foo = Foo(); + foo.bar(() { + final a = 1; + return; + }); + return; +} + ''', + [lint(130, 7), lint(146, 7)], + ); + } +} diff --git a/test/lints/no_equal_then_else/no_equal_then_else_rule_test.dart b/test/lints/no_equal_then_else/no_equal_then_else_rule_test.dart new file mode 100644 index 00000000..8eecbb3f --- /dev/null +++ b/test/lints/no_equal_then_else/no_equal_then_else_rule_test.dart @@ -0,0 +1,88 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/no_equal_then_else/no_equal_then_else_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(NoEqualThenElseRuleTest); + }); +} + +@reflectiveTest +class NoEqualThenElseRuleTest extends AnalysisRuleTest { + @override + void setUp() { + rule = NoEqualThenElseRule(); + super.setUp(); + } + + @override + String get analysisRule => rule.name; + + void test_reports_then_and_else_equal() async { + await assertDiagnostics( + r''' +void fun() { + final _valueA = 1; + final _valueB = 2; + + int _result = 0; + + if (_valueA == 1) { + _result = _valueA; + } else { + _result = _valueA; + } +} +''', + [lint(78, 80)], + ); + } + + void test_does_not_report_then_and_else_different() async { + await assertNoDiagnostics(r''' +void fun() { + final _valueA = 1; + final _valueB = 2; + + int _result = 0; + + if (_valueA == 1) { + _result = _valueA; + } else { + _result = _valueB; + } +} +'''); + } + + void test_reports_conditional_expression_then_and_else_equal() async { + await assertDiagnostics( + r''' +void fun() { + final _valueA = 1; + final _valueB = 2; + + int _result = 0; + + _result = _valueA == 2 ? _valueA : _valueA; +} +''', + [lint(88, 32)], + ); + } + + void + test_does_not_report_conditional_expression_then_and_else_different() async { + await assertNoDiagnostics(r''' +void fun() { + final _valueA = 1; + final _valueB = 2; + + int _result = 0; + + _result = _valueA == 2 ? _valueA : _valueB; +} +'''); + } +} diff --git a/test/lints/prefer_early_return/prefer_early_return_rule_test.dart b/test/lints/prefer_early_return/prefer_early_return_rule_test.dart new file mode 100644 index 00000000..2e52493a --- /dev/null +++ b/test/lints/prefer_early_return/prefer_early_return_rule_test.dart @@ -0,0 +1,494 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(PreferEarlyReturnRuleTest); + }); +} + +@reflectiveTest +class PreferEarlyReturnRuleTest extends AnalysisRuleTest { + @override + void setUp() { + rule = PreferEarlyReturnRule(); + super.setUp(); + } + + @override + String get analysisRule => PreferEarlyReturnRule.lintName; + + void test_reports_if_as_only_statement_in_function() async { + await assertDiagnostics( + r''' +void test(bool a) { + if (a) { + print('hello'); + } +} +''', + [lint(22, 32)], + ); + } + + void test_reports_if_with_return() async { + await assertDiagnostics( + r''' +void test(bool a) { + if (a) { + print('hello'); + } + + return; +} +''', + [lint(22, 32)], + ); + } + + void test_does_not_report_if_with_return_value() async { + await assertNoDiagnostics( + r''' +int test(bool a) { + if (a) { + print('hello'); + } + + return 1; +} +''', + ); + } + + void test_reports_nested_if_as_only_statement() async { + await assertDiagnostics( + r''' +void test(bool a, bool b) { + if (a) { + if (b) { + print('nested'); + } + } +} +''', + [ + lint(30, 54), + ], + ); + } + + void test_does_not_report_nested_if_with_return_value() async { + await assertNoDiagnostics( + r''' +int test(bool a, bool b) { + if (a) { + if (b) { + print('nested'); + } + } + + return 1; +} +''', + ); + } + + void test_reports_nested_3_if_as_only_statement() async { + await assertDiagnostics( + r''' +void test(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c){ + print('nested'); + } + } + } +} +''', + [ + lint(38, 78), + ], + ); + } + + void test_reports_nested_3_with_return() async { + await assertDiagnostics( + r''' +void test(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c){ + print('nested'); + } + } + } + return; +} +''', + [ + lint(38, 78), + ], + ); + } + + void test_does_not_report_if_else() async { + await assertNoDiagnostics( + r''' +void test(bool a) { + if (a) { + print('hello'); + } else { + print('hello'); + } +} +''', + ); + } + + void test_does_not_report_if_else_return() async { + await assertNoDiagnostics( + r''' +void test(bool a) { + if (a) { + print('hello'); + } else { + return; + } +} +''', + ); + } + + void test_does_not_report_nested_if_else() async { + await assertNoDiagnostics( + r''' +void test(bool a, bool b) { + if (a) { + if(b){ + print('hello'); + } + } else { + print('hello'); + } +} +''', + ); + } + + void test_reports_inner_if_else() async { + await assertDiagnostics( + r''' +void test(bool a, bool b) { + if (a) { + if(b){ + print('hello'); + } + else { + print('hello'); + } + } +} +''', + [ + lint(30, 90), + ], + ); + } + + void test_reports_on_three_if() async { + await assertDiagnostics( + r''' +void threeIf(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c) { + print('hello'); + } + } + } +}''', + [lint(41, 74)], + ); + } + + void test_does_not_report_nested_3_with_else_1() async { + await assertNoDiagnostics( + r''' +void test(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c){ + print('nested'); + } + } + } else{ + print('hello'); + } +} +''', + ); + } + + void test_reports_nested_3_with_else_2() async { + await assertDiagnostics( + r''' +void test(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c) { + print('nested'); + } + } else { + print('nested'); + } + } +} +''', + [ + lint(38, 115), + ], + ); + } + + void test_reports_nested_3_with_else_3() async { + await assertDiagnostics( + r''' +void test(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c) { + print('nested'); + } + else { + print('nested'); + } + } + } +} +''', + [ + lint(38, 126), + ], + ); + } + + void test_reports_2_sequencial_if() async { + await assertDiagnostics( + r''' +void test(bool a, bool b) { + if (a) return; + if (b) { + print('gello'); + } +} +''', + [ + lint(47, 32), + ], + ); + } + + void test_does_not_report_2_sequencial_if_with_return() async { + await assertNoDiagnostics( + r''' +void test(bool a, bool b) { + if (a) return; + if (b) return; + + return; +} +''', + ); + } + + void test_reports_2_sequencial_if_with_return_2() async { + await assertDiagnostics( + r''' +void test(bool a, bool b) { + if (a) return; + if (b) { + print('hello'); + } + + return; +} +''', + [ + lint(47, 32), + ], + ); + } + + void test_does_not_report_2_sequencial_with_following_statement() async { + await assertNoDiagnostics( + r''' +void test(bool a, bool b) { + if (a) return; + if (b) { + print('hello'); + } + + print('after'); +} +''', + ); + } + + void test_reports_2_sequencial_if_with_something() async { + await assertDiagnostics( + r''' +void test(bool a, bool b) { + if (a) { + print('hello'); + } + if (b) { + print('hello'); + } +} +''', + [ + lint(65, 32), + ], + ); + } + + void test_reports_3_sequencial_if_with_return() async { + await assertDiagnostics( + r''' +void test(bool a, bool b, bool c) { + if (a) return; + if (b) return; + if (c) { + print('hello'); + } + + return; +} +''', + [ + lint(72, 32), + ], + ); + } + + void test_reports_3_sequencial_if_with_something_2() async { + await assertDiagnostics( + r''' +void test(bool a, bool b, bool c) { + if (a) return; + if (b) { + print('hello'); + } + if (c) { + print('hello'); + } +} +''', + [ + lint(90, 32), + ], + ); + } + + void test_does_not_report_if_throw_with_return() async { + await assertNoDiagnostics( + r''' +void test(bool a) { + if (a) { + throw ''; + } + + return; +} +''', + ); + } + + void test_does_not_report_if_else_throw() async { + await assertNoDiagnostics( + r''' +void test(bool a) { + if (a) { + print('hello'); + } else { + throw ''; + } +} +''', + ); + } + + void test_reports_2_sequencial_if_throw() async { + await assertDiagnostics( + r''' +void test(bool a, bool b) { + if (a) throw ''; + if (b) { + print('hello'); + } +} +''', + [ + lint(49, 32), + ], + ); + } + + void test_reports_2_sequencial_if_throw_with_return() async { + await assertDiagnostics( + r''' +void test(bool a, bool b) { + if (a) throw ''; + if (b) { + print('hello'); + } + + return; +} +''', + [ + lint(49, 32), + ], + ); + } + + void test_reports_3_sequencial_if_throw_with_return() async { + await assertDiagnostics( + r''' +void test(bool a, bool b, bool c) { + if (a) throw ''; + if (b) throw ''; + if (c) { + print('hello'); + } + + return; +} +''', + [ + lint(76, 32), + ], + ); + } + + void test_reports_3_sequencial_if_throw_with_something() async { + await assertDiagnostics( + r''' +void test(bool a, bool b, bool c) { + if (a) throw ''; + if (b) { + print('hello'); + } + if (c) { + print('hello'); + } +} +''', + [ + lint(92, 32), + ], + ); + } +} diff --git a/test/lints/proper_super_calls/proper_super_calls_rule_test.dart b/test/lints/proper_super_calls/proper_super_calls_rule_test.dart new file mode 100644 index 00000000..8969e73c --- /dev/null +++ b/test/lints/proper_super_calls/proper_super_calls_rule_test.dart @@ -0,0 +1,170 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(ProperSuperCallsRuleTest); + }); +} + +@reflectiveTest +class ProperSuperCallsRuleTest extends AnalysisRuleTest { + @override + void setUp() { + final flutter = newPackage('flutter'); + flutter.addFile( + 'lib/src/widgets/framework.dart', + r''' +abstract class StatefulWidget {} + +abstract class State { + void initState() {} + void dispose() {} +} +''', + ); + + rule = ProperSuperCallsRule(); + super.setUp(); + } + + @override + String get analysisRule => ProperSuperCallsRule.lintName; + + void test_initState_reports_when_super_not_first() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + @override + void initState() { + print('Bad'); + super.initState(); + } +} +''', + [lint(125, 9)], + ); + } + + void test_dispose_reports_when_super_not_last() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + @override + void dispose() { + super.dispose(); + print('Bad'); + } +} +''', + [lint(125, 7)], + ); + } + + void test_reports_even_without_explicit_override_annotation() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + void initState() { + print('Bad'); + super.initState(); + } +} +''', + [lint(113, 9)], + ); + } + + void test_reports_empty_body_missing_super() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + @override + void initState() {} +} +''', + [lint(125, 9)], + ); + } + + void test_reports_when_wrong_super_method_is_called() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + @override + void initState() { + super.dispose(); + print('Bad'); + } +} +''', + [lint(125, 9)], + ); + } + + void test_reports_in_deep_inheritance() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +abstract class BaseState extends State {} + +class MyWidgetState extends BaseState { + @override + void dispose() { + super.dispose(); + print('Bad'); + } +} +''', + [lint(172, 7)], + ); + } + + void test_no_report_for_correct_usage() async { + await assertNoDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + @override + void initState() { + super.initState(); + print('Good'); + } + + @override + void dispose() { + print('Good'); + super.dispose(); + } +} +''', + ); + } + + void test_no_report_for_other_methods() async { + await assertNoDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + void build() { + super.initState(); + } +} +''', + ); + } +} diff --git a/test/src/common/parameter_parser/analysis_options_loader_test.dart b/test/src/common/parameter_parser/analysis_options_loader_test.dart new file mode 100644 index 00000000..a727136f --- /dev/null +++ b/test/src/common/parameter_parser/analysis_options_loader_test.dart @@ -0,0 +1,225 @@ +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/file_system/file_system.dart'; +import 'package:analyzer/workspace/workspace.dart'; +import 'package:analyzer_testing/src/analysis_rule/pub_package_resolution.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; +import 'package:test/test.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AnalysisOptionsLoaderTest); + }); +} + +@reflectiveTest +class AnalysisOptionsLoaderTest extends PubPackageResolutionTest { + // TODO: use actual [Rule.lintName] after migrating to analyzer_server_plugin + // Can't be used right now because they have compile errors + static const _mockRuleThatNeedsConfigName = 'mock_rule_that_needs_config'; + static const _mockRule2Name = 'mock_rule_2'; + static const _cyclomaticComplexityName = 'cyclomatic_complexity'; + + static const _mockAnalysisOptionsContent = ''' +plugins: + solid_lints: + diagnostics: + $_mockRuleThatNeedsConfigName: + abc: def + $_mockRule2Name: + foo: bar + exclude: + - class_name: MockClass + method_name: mockMethod + $_cyclomaticComplexityName: + max_complexity: 10 + exclude: + - class_name: MockClass + method_name: mockMethod + - method_name: mockMethod2 + '''; + static const _mockDifferentAnalysisOptionsContent = ''' +plugins: + solid_lints: + diagnostics: + $_mockRuleThatNeedsConfigName: + abc: ghi + $_mockRule2Name: + foo: baz + exclude: + - class_name: MockOtherClass + method_name: mockOtherMethod + $_cyclomaticComplexityName: + max_complexity: 20 + exclude: + - class_name: MockOtherClass + method_name: mockOtherMethod + - method_name: mockOtherMethod2 + '''; + + late AnalysisOptionsLoader analysisOptionsLoader; + late RuleContext mockRuleContext; + + @override + void setUp() { + super.setUp(); + + analysisOptionsLoader = + AnalysisOptionsLoader(resourceProvider: resourceProvider); + mockRuleContext = _createMockContextForPackage(testPackageRootPath); + + _writeMockAnalysisOptionsYamlFile(); + + analysisOptionsLoader.loadRulesOptionsFromContext(mockRuleContext); + } + + void _writeMockAnalysisOptionsYamlFile() { + newAnalysisOptionsYamlFile( + testPackageRootPath, + _mockAnalysisOptionsContent, + ); + } + + void test_cached_response_is_scoped_to_package_and_rule() { + const otherPackageRootPath = '/home/other'; + + newFolder(otherPackageRootPath); + newPubspecYamlFile(otherPackageRootPath, 'name: other'); + newAnalysisOptionsYamlFile( + otherPackageRootPath, + _mockDifferentAnalysisOptionsContent, + ); + + for (final ruleName in [ + _mockRuleThatNeedsConfigName, + _mockRule2Name, + _cyclomaticComplexityName + ]) { + final currentPackageOptions = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + ruleName, + ); + final otherPackageOptions = analysisOptionsLoader.getRuleOptions( + _createMockContextForPackage(otherPackageRootPath), + ruleName, + ); + + expect( + currentPackageOptions, + isNot(equals(otherPackageOptions)), + ); + } + } + + void test_each_rule_gets_its_options() { + analysisOptionsLoader.loadRulesOptionsFromContext(mockRuleContext); + + final mockRuleThatNeedsConfigOptions = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _mockRuleThatNeedsConfigName, + ); + final mockRule2Options = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _mockRule2Name, + ); + final cyclomaticComplexityOptions = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _cyclomaticComplexityName, + ); + + expect(mockRuleThatNeedsConfigOptions, isNotNull); + expect(mockRuleThatNeedsConfigOptions, {'abc': 'def'}); + + expect(mockRule2Options, isNotNull); + expect(mockRule2Options, { + 'foo': 'bar', + 'exclude': [ + {'class_name': 'MockClass', 'method_name': 'mockMethod'}, + ] + }); + + expect(cyclomaticComplexityOptions, isNotNull); + expect(cyclomaticComplexityOptions, { + 'max_complexity': 10, + 'exclude': [ + {'class_name': 'MockClass', 'method_name': 'mockMethod'}, + {'method_name': 'mockMethod2'}, + ] + }); + } + + void test_invalidates_cache_when_analysis_options_changed() { + final initialOptions = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _mockRuleThatNeedsConfigName, + ); + + newAnalysisOptionsYamlFile( + testPackageRootPath, + _mockDifferentAnalysisOptionsContent, + ); + analysisOptionsLoader.loadRulesOptionsFromContext(mockRuleContext); + + final updatedOptions = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _mockRuleThatNeedsConfigName, + ); + + expect(initialOptions, {'abc': 'def'}); + expect(updatedOptions, {'abc': 'ghi'}); + expect(updatedOptions, isNot(same(initialOptions))); + } + + void test_loads_and_parses_rule_options_from_yaml_file() { + analysisOptionsLoader.loadRulesOptionsFromContext(mockRuleContext); + + final options = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _mockRuleThatNeedsConfigName, + ); + + expect(options, isNotNull); + expect(options, {'abc': 'def'}); + } + + void test_returns_cached_response_for_same_rule_name() { + analysisOptionsLoader.loadRulesOptionsFromContext(mockRuleContext); + + final firstOptions = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _mockRuleThatNeedsConfigName, + ); + final secondOptions = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _mockRuleThatNeedsConfigName, + ); + + expect(secondOptions, same(firstOptions)); + } + + RuleContext _createMockContextForPackage(String packageRootPath) { + return _TestRuleContext( + _TestWorkspacePackage(getFolder(packageRootPath)), + ); + } +} + +class _TestRuleContext implements RuleContext { + @override + final WorkspacePackage? package; + + _TestRuleContext(this.package); + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +class _TestWorkspacePackage implements WorkspacePackage { + @override + final Folder root; + + _TestWorkspacePackage(this.root); + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} diff --git a/test/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule_test.dart b/test/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule_test.dart new file mode 100644 index 00000000..2149a1c0 --- /dev/null +++ b/test/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule_test.dart @@ -0,0 +1,131 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidFinalWithGetterRuleTest); + }); +} + +@reflectiveTest +class AvoidFinalWithGetterRuleTest extends AnalysisRuleTest { + @override + void setUp() { + rule = AvoidFinalWithGetterRule(); + super.setUp(); + } + + Future test_reports_on_getter_with_same_name_as_field() async { + await assertDiagnostics( + r''' +class Test { + final int _myField = 0; + + int get myField => _myField; +} +''', + [lint(44, 28)], + ); + } + + Future test_reports_on_getter_with_different_name_from_field() async { + await assertDiagnostics( + r''' +class Test { + final int _myField = 0; + + int get myFieldInt => _myField; +} +''', + [lint(44, 31)], + ); + } + + Future test_reports_on_static_getter_with_private_field() async { + await assertDiagnostics( + r''' +class Test { + static final int _myField = 0; + + static int get myField => _myField; +} +''', + [lint(51, 35)], + ); + } + + Future test_reports_on_getter_with_this_property_access() async { + await assertDiagnostics( + r''' +class Test { + final int _myField = 0; + + int get myField => this._myField; +} +''', + [lint(44, 33)], + ); + } + + Future + test_reports_on_block_body_getter_returning_private_field() async { + await assertDiagnostics( + r''' +class Test { + final int _myField = 0; + + int get myField { + return _myField; + } +} +''', + [lint(44, 42)], + ); + } + + Future test_does_not_report_on_getter_that_contains_logic() async { + await assertNoDiagnostics(r''' +class Test { + final int _myField = 0; + final int _myField2 = 1; + final int _myField3 = 2; + + int get myField => _myField + 1; + + int get myField2 => this._myField2 + 1; + + int get myField3 { + return this._myField3 + 1; + } +} +'''); + } + + Future test_does_not_report_on_public_final_field() async { + await assertNoDiagnostics(r''' +class Test { + final int myField = 0; +} +'''); + } + + Future + test_does_not_report_on_getter_returning_mutable_private_field() async { + await assertNoDiagnostics(r''' +class Test { + int _myField = 0; + int _myField2 = 1; + int _myField3 = 2; + + int get myField => _myField; + + int get myField2 => this._myField2; + + int get myField3 { + return _myField3; + } +} +'''); + } +} diff --git a/test/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule_test.dart b/test/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule_test.dart new file mode 100644 index 00000000..f780c3df --- /dev/null +++ b/test/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule_test.dart @@ -0,0 +1,296 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:analyzer_testing/utilities/utilities.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; +import 'package:solid_lints/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart'; +import 'package:solid_lints/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidReturningWidgetsRuleTest); + }); +} + +@reflectiveTest +class AvoidReturningWidgetsRuleTest extends AnalysisRuleTest { + static const _importFlutterWidgets = "import 'package:flutter/widgets.dart';"; + static const _mockFlutterWidgetsContent = ''' +abstract class Widget { + final String key; + + const Widget({required this.key}); +} + +class StatelessWidget implements Widget { + const StatelessWidget({super.key}); + + @override + Widget build(BuildContext context); +} + +class StatefulWidget implements Widget { + const StatefulWidget({super.key}); + + @override + Widget build(BuildContext context); +} + +abstract interface class BuildContext {} + +class Placeholder extends StatelessWidget { + const Placeholder({super.key}); + + @override + Widget build(BuildContext context) => throw 'unimplemented'; +} + +class SizedBox extends Widget { + final Widget? child; + + const SizedBox({this.child}); + + @override + Widget build(BuildContext context) => child ?? const SizedBox(); +} + +class BoxDecoration extends Widget { + const BoxDecoration(); + + @override + Widget build(BuildContext context) => throw 'unimplemented'; +} + +class DecoratedBox extends Widget { + const DecoratedBox({required this.decoration}); + + final BoxDecoration decoration; + + @override + Widget build(BuildContext context) => throw 'unimplemented'; +} +'''; + static const _mockAnalysisOptionsContent = ''' +plugins: + solid_lints: + diagnostics: + avoid_returning_widgets: + exclude: + - class_name: ExcludeWidget + method_name: excludeWidgetMethod + - method_name: excludeMethod + '''; + + void _addBaseWidgetFile() { + newFile('$testPackageLibPath/base_widget.dart', ''' +$_importFlutterWidgets +class BaseWidget extends StatelessWidget { + const BaseWidget({super.key}); + + Widget get box => SizedBox(); + + Widget decoratedBox() => DecoratedBox(decoration: BoxDecoration()); + + set box(Widget value) { + throw 'unimplemented'; + } +} +'''); + } + + @override + void setUp() { + rule = AvoidReturningWidgetsRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + parametersParser: AvoidReturningWidgetsParameters.fromJson, + ); + newPackage('flutter') + ..addFile('lib/widgets.dart', _mockFlutterWidgetsContent); + super.setUp(); + + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +$_mockAnalysisOptionsContent''', + ); + } + + Future test_reports_on_static_function() async { + await assertDiagnostics( + ''' +$_importFlutterWidgets + +Widget avoidReturningWidgets() => const SizedBox(); + +Widget build() { + return SizedBox(); +} +''', + [lint(40, 51), lint(93, 39)], + ); + } + + Future test_reports_on_methods() async { + await assertDiagnostics( + ''' +$_importFlutterWidgets + +class BaseWidget extends StatelessWidget { + const BaseWidget({super.key}); + + Widget decoratedBox() { + return DecoratedBox(decoration: BoxDecoration()); + } +} +''', + [lint(119, 81)], + ); + } + + Future test_reports_on_getters_but_not_setters() async { + await assertDiagnostics( + ''' +$_importFlutterWidgets + +class BaseWidget extends StatelessWidget { + const BaseWidget({super.key}); + + Widget get box => SizedBox(); + + set box(Widget value) { + throw 'unimplemented'; + } +} +''', + [lint(119, 29)], + ); + } + + Future test_reports_on_private_members() async { + _addBaseWidgetFile(); + + await assertDiagnostics( + ''' +$_importFlutterWidgets +import 'base_widget.dart'; + +class MyWidget extends BaseWidget { + const MyWidget({super.key}); + + Widget _test1() => const SizedBox(); + + Widget _test2() { + return const SizedBox( + child: SizedBox(), + ); + } + + Widget get _test3 => const SizedBox(); + + @override + Widget decoratedBox() { + return super.decoratedBox(); + } + + @override + Widget get box => SizedBox(); + + @override + Widget build(BuildContext context) { + return const SizedBox(); + } +} +''', + [ + lint(137, 36), + lint(177, 80), + lint(261, 38), + ], + ); + } + + Future test_does_not_report_on_overridden_members() async { + _addBaseWidgetFile(); + + // Shouldn't report even if not annotated with @override + await assertNoDiagnostics( + ''' +$_importFlutterWidgets +import 'base_widget.dart'; + +class MyWidget extends BaseWidget { + const MyWidget({super.key}); + + Widget decoratedBox() { + return super.decoratedBox(); + } + + Widget get box => SizedBox(); + + Widget build(BuildContext context) { + return const SizedBox(); + } +} +''', + ); + } + + Future test_does_not_report_on_excluded() async { + await assertNoDiagnostics( + ''' +$_importFlutterWidgets + +SizedBox excludeMethod() => const SizedBox(); + +class ExcludeWidget extends StatelessWidget { + const ExcludeWidget({super.key}); + + @override + Widget build(BuildContext context) { + return const Placeholder(); + } + + Widget excludeWidgetMethod() => const SizedBox(); +} + +''', + ); + } + + Future test_reports_on_non_matching_excluded() async { + await assertDiagnostics( + ''' +$_importFlutterWidgets + +SizedBox excludeMethod() => const SizedBox(); + +class ExcludeWidget extends StatelessWidget { + const ExcludeWidget({super.key}); + + @override + Widget build(BuildContext context) { + return const Placeholder(); + } + + Widget notExcludeWidgetMethod() => const Placeholder(); +} + +class NotExcludeWidget extends StatelessWidget { + const NotExcludeWidget({super.key}); + + @override + Widget build(BuildContext context) { + return const Placeholder(); + } + + Widget excludeWidgetMethod() => const SizedBox(); +} +''', + [ + lint(260, 55), + lint(498, 49), + ], + ); + } +} diff --git a/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart new file mode 100644 index 00000000..d8b1d643 --- /dev/null +++ b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart @@ -0,0 +1,134 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:analyzer_testing/utilities/utilities.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(FunctionLinesOfCodeRuleTest); + }); +} + +@reflectiveTest +class FunctionLinesOfCodeRuleTest extends AnalysisRuleTest { + static const _mockAnalysisOptionsContent = ''' +plugins: + solid_lints: + diagnostics: + function_lines_of_code: + max_lines: 4 + exclude: + - class_name: ClassWithLongMethods + method_name: longMethodExcluded + - method_name: longMethodExcludedByDeclarationName + - method_name: longFunctionExcluded + - method_name: longFunctionExcludedByDeclarationName + - longMethodExcludedByString + '''; + + @override + void setUp() { + rule = FunctionLinesOfCodeRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + parametersParser: FunctionLinesOfCodeParameters.fromJson, + ); + super.setUp(); + + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +$_mockAnalysisOptionsContent''', + ); + } + + Future test_reports_when_lines_exceed_threshold() async { + await assertDiagnostics( + r''' +int longFunction() { + var i = 0; + i++; + i++; + i++; + + return i; +} +''', + [lint(0, 69)], + ); + } + + Future test_does_not_report_when_lines_within_threshold() async { + await assertNoDiagnostics(r''' +int shortFunction() { + var i = 0; + i++; + i++; + + return i; +} +'''); + } + + Future test_does_not_report_on_excluded_method() async { + await assertNoDiagnostics(r''' +class ClassWithLongMethods { + int longMethodExcluded() { + var i = 0; + i++; + i++; + i++; + + return i; + } +} +'''); + } + + Future test_does_not_report_on_excluded_top_level_function() async { + await assertNoDiagnostics(r''' +int longFunctionExcluded() { + var i = 0; + i++; + i++; + i++; + + return i; +} +'''); + } + + Future test_reports_on_anonymous_functions() async { + await assertDiagnostics( + r''' +final longAnonymousFunction = () { + var i = 0; + i++; + i++; + i++; + + return i; +}; +''', + [lint(30, 53)], + ); + } + + Future test_does_not_report_on_method_excluded_by_string() async { + await assertNoDiagnostics(r''' +class SomeClass { + int longMethodExcludedByString() { + var i = 0; + i++; + i++; + i++; + + return i; + } +} +'''); + } +}