diff --git a/README.md b/README.md index 8e53dbf2..09038a7a 100644 --- a/README.md +++ b/README.md @@ -50,12 +50,13 @@ You can customize individual rule settings in your `analysis_options.yaml`: ```yaml plugins: - solid_lints: - version: - diagnostics: - cyclomatic_complexity: - max_complexity: 10 - avoid_non_null_assertion: true + solid_lints: + +solid_lints: + diagnostics: + cyclomatic_complexity: + max_complexity: 10 + avoid_non_null_assertion: true ``` # Badge diff --git a/example/analysis_options.yaml b/example/analysis_options.yaml index c38292e1..5d65a85c 100644 --- a/example/analysis_options.yaml +++ b/example/analysis_options.yaml @@ -3,28 +3,30 @@ include: package:solid_lints/analysis_options.yaml plugins: solid_lints: path: ../ - diagnostics: - cyclomatic_complexity: - max_complexity: 4 - number_of_parameters: - max_parameters: 2 - function_lines_of_code: - max_lines: 50 - avoid_non_null_assertion: true - avoid_late_keyword: true - avoid_global_state: true - avoid_returning_widgets: true - avoid_unnecessary_setstate: true - double_literal_format: true - avoid_unnecessary_type_assertions: true - avoid_debug_print_in_release: true - avoid_using_api: - severity: info - entries: - - class_name: Future - identifier: wait - source: dart:async - reason: >- - Future.wait should be avoided because it loses type safety for - the results. Use a Record's `wait` method instead. - severity: warning + +solid_lints: + diagnostics: + cyclomatic_complexity: + max_complexity: 4 + number_of_parameters: + max_parameters: 2 + function_lines_of_code: + max_lines: 50 + avoid_non_null_assertion: true + avoid_late_keyword: true + avoid_global_state: true + avoid_returning_widgets: true + avoid_unnecessary_setstate: true + double_literal_format: true + avoid_unnecessary_type_assertions: true + avoid_debug_print_in_release: true + avoid_using_api: + severity: info + entries: + - class_name: Future + identifier: wait + source: dart:async + reason: >- + Future.wait should be avoided because it loses type safety for + the results. Use a Record's `wait` method instead. + severity: warning diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index 16b58eb2..dfb16252 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -96,7 +96,8 @@ solid_lints: newline_before_return: true no_empty_block: true no_equal_then_else: true - prefer_early_return: true + # Disabled by default for now. Will be considered for future inclusion. + prefer_early_return: false no_magic_number: allowed_in_widget_params: true diff --git a/lib/src/common/parameter_parser/analysis_options_loader.dart b/lib/src/common/parameter_parser/analysis_options_loader.dart index 104d8aa3..88be58d6 100644 --- a/lib/src/common/parameter_parser/analysis_options_loader.dart +++ b/lib/src/common/parameter_parser/analysis_options_loader.dart @@ -72,6 +72,34 @@ class AnalysisOptionsLoader { ) ?? false; + /// Checks if a file is excluded by the analysis options configuration. + bool isFileExcluded(RuleContext context) { + final targetPath = (context.currentUnit ?? context.definingUnit).file.path; + + return isFileExcludedForFile(targetPath); + } + + /// Checks if a specific file at [filePath] is excluded by the nearest + /// analysis options file. + bool isFileExcludedForFile(String filePath) { + final pathContext = _resourceProvider.pathContext; + if (!pathContext.isAbsolute(filePath)) return false; + + final dirPath = pathContext.dirname(filePath); + final yamlPath = _findNearestAnalysisOptionsFilePath(dirPath); + if (yamlPath == null) return false; + + _loadRulesOptionsIfNewer(yamlPath); + final rootDir = pathContext.dirname(yamlPath); + + return _rulesCache[yamlPath]?.isPathExcluded( + filePath, + pathContext, + rootDir, + ) ?? + false; + } + /// Loads lint rules from the analysis options file for all rules /// using the provided [RuleContext]. void loadRulesOptionsFromContext(RuleContext context) => @@ -84,8 +112,11 @@ class AnalysisOptionsLoader { RuleContext context, T Function(String) f, ) { - final filePath = context.definingUnit.file.path; - final dirPath = _resourceProvider.pathContext.dirname(filePath); + final filePath = (context.currentUnit ?? context.definingUnit).file.path; + final pathContext = _resourceProvider.pathContext; + if (!pathContext.isAbsolute(filePath)) return null; + + final dirPath = pathContext.dirname(filePath); final yamlPath = _findNearestAnalysisOptionsFilePath(dirPath); if (yamlPath == null) return null; @@ -107,11 +138,14 @@ class AnalysisOptionsLoader { modificationStamp: modificationStamp, rules: rulesData.rules, disabledRules: rulesData.disabledRules, + excludedPatterns: rulesData.excludedPatterns, ); } String? _findNearestAnalysisOptionsFilePath(String startDirectoryPath) { final pathContext = _resourceProvider.pathContext; + if (!pathContext.isAbsolute(startDirectoryPath)) return null; + var currentDirectoryPath = startDirectoryPath; while (currentDirectoryPath.isNotEmpty) { diff --git a/lib/src/common/parameter_parser/analysis_options_parser.dart b/lib/src/common/parameter_parser/analysis_options_parser.dart index 0e002747..585e52b8 100644 --- a/lib/src/common/parameter_parser/analysis_options_parser.dart +++ b/lib/src/common/parameter_parser/analysis_options_parser.dart @@ -36,6 +36,7 @@ class AnalysisOptionsParser { final mergedRules = >{}; final disabledRules = {}; + final excludedPatterns = {}; final nextSeenPaths = {...seenPaths, path}; _resolveAndMergeIncludes( @@ -44,11 +45,17 @@ class AnalysisOptionsParser { nextSeenPaths, mergedRules, disabledRules, + excludedPatterns, ); _parseRuleOptions(yaml, mergedRules, disabledRules); _parseSuppressedErrors(yaml, mergedRules, disabledRules); + _parseExcludedPatterns(yaml, excludedPatterns); - return RulesData(rules: mergedRules, disabledRules: disabledRules); + return RulesData( + rules: mergedRules, + disabledRules: disabledRules, + excludedPatterns: excludedPatterns, + ); } Map? _parseYaml(File file) { @@ -79,6 +86,7 @@ class AnalysisOptionsParser { Set seenPaths, Map> mergedRules, Set disabledRules, + Set excludedPatterns, ) { final includeOption = yaml['include']; switch (includeOption) { @@ -89,6 +97,7 @@ class AnalysisOptionsParser { seenPaths, mergedRules, disabledRules, + excludedPatterns, ); case List(): for (final include in includeOption) { @@ -99,6 +108,7 @@ class AnalysisOptionsParser { seenPaths, mergedRules, disabledRules, + excludedPatterns, ); } } @@ -111,6 +121,7 @@ class AnalysisOptionsParser { Set seenPaths, Map> mergedRules, Set disabledRules, + Set excludedPatterns, ) { final includedFile = _resolveIncludedFile(baseFile, includePath); if (includedFile == null) return; @@ -124,6 +135,7 @@ class AnalysisOptionsParser { }; } disabledRules.addAll(includedData.disabledRules); + excludedPatterns.addAll(includedData.excludedPatterns); } File? _resolveIncludedFile(File baseFile, String includePath) { @@ -228,4 +240,21 @@ class AnalysisOptionsParser { } } } + + /// Parses file exclusion patterns configured under `analyzer: exclude:`. + void _parseExcludedPatterns( + Map yaml, + Set excludedPatterns, + ) { + final analyzer = yaml['analyzer']; + if (analyzer is! Map) return; + + final exclude = analyzer['exclude']; + switch (exclude) { + case String(): + excludedPatterns.add(exclude); + case Iterable(): + excludedPatterns.addAll(exclude.whereType()); + } + } } diff --git a/lib/src/common/parameter_parser/cached_package_rules.dart b/lib/src/common/parameter_parser/cached_package_rules.dart index d9bee3bb..0f1eb20b 100644 --- a/lib/src/common/parameter_parser/cached_package_rules.dart +++ b/lib/src/common/parameter_parser/cached_package_rules.dart @@ -1,3 +1,7 @@ +import 'package:glob/glob.dart'; +import 'package:path/path.dart' as p; +import 'package:solid_lints/src/utils/function_utils.dart'; + /// Cached rules for a dart package class CachedPackageRules { /// The last modification stamp of the analysis options file @@ -9,10 +13,44 @@ class CachedPackageRules { /// Rules that are explicitly disabled final Set disabledRules; + /// Patterns of excluded files + final Set excludedPatterns; + + final List _compiledGlobs; + final Map _pathExclusionCache = {}; + /// Creates an instance of [CachedPackageRules] - const CachedPackageRules({ + CachedPackageRules({ required this.modificationStamp, required this.rules, required this.disabledRules, - }); + required this.excludedPatterns, + }) : _compiledGlobs = _compileGlobs(excludedPatterns); + + static List _compileGlobs(Set patterns) => patterns + .map( + (pattern) => FunctionUtils.tryOrNull( + () => Glob(pattern, context: p.posix), + ), + ) + .nonNulls + .toList(); + + /// Checks if [filePath] matches any of the excluded patterns. + bool isPathExcluded( + String filePath, + p.Context pathContext, + String rootDir, + ) { + if (excludedPatterns.isEmpty) return false; + + return _pathExclusionCache.putIfAbsent(filePath, () { + final relativePath = pathContext.isWithin(rootDir, filePath) + ? pathContext.relative(filePath, from: rootDir) + : filePath; + final normalizedPath = p.posix.joinAll(pathContext.split(relativePath)); + + return _compiledGlobs.any((glob) => glob.matches(normalizedPath)); + }); + } } diff --git a/lib/src/common/parameter_parser/rules_data.dart b/lib/src/common/parameter_parser/rules_data.dart index d7e3c54b..2f158ce8 100644 --- a/lib/src/common/parameter_parser/rules_data.dart +++ b/lib/src/common/parameter_parser/rules_data.dart @@ -6,9 +6,19 @@ class RulesData { /// The set of explicitly disabled rules. final Set disabledRules; + /// The set of file paths or glob patterns excluded from analysis. + final Set excludedPatterns; + /// Creates a new instance of [RulesData]. - const RulesData({required this.rules, required this.disabledRules}); + const RulesData({ + required this.rules, + required this.disabledRules, + required this.excludedPatterns, + }); /// Creates a new empty instance of [RulesData]. - const RulesData.empty() : rules = const {}, disabledRules = const {}; + const RulesData.empty() + : rules = const {}, + disabledRules = const {}, + excludedPatterns = const {}; } diff --git a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart index f96785d5..28f48542 100644 --- a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart +++ b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart @@ -4,6 +4,7 @@ import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart'; import 'package:solid_lints/src/models/solid_lint_rule.dart'; +import 'package:solid_lints/src/utils/ignore_matcher.dart'; /// A lint rule that detects duplicated code blocks (clones) across the project. /// @@ -99,17 +100,16 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// avoid_duplicate_code: -/// min_tokens: 30 -/// ignore_literals: false -/// ignore_identifiers: true -/// check_blocks: true -/// exclude: -/// - method_name: initState -/// - method_name: dispose +/// solid_lints: +/// diagnostics: +/// avoid_duplicate_code: +/// min_tokens: 30 +/// ignore_literals: false +/// ignore_identifiers: true +/// check_blocks: true +/// exclude: +/// - method_name: initState +/// - method_name: dispose /// ``` class AvoidDuplicateCodeRule extends SolidLintRule { @@ -136,6 +136,9 @@ class AvoidDuplicateCodeRule parametersParser: AvoidDuplicateCodeParameters.fromJson, ); + /// The ignore matcher instance for this rule. + final ignoreMatcher = IgnoreMatcher(lintName); + @override void registerNodeProcessors( RuleVisitorRegistry registry, @@ -147,13 +150,16 @@ class AvoidDuplicateCodeRule getParametersForContext(context) ?? AvoidDuplicateCodeParameters.empty(); + final currentUnit = context.currentUnit ?? context.definingUnit; final visitor = AvoidDuplicateCodeVisitor( this, parameters, - filePath: context.definingUnit.file.path, - modificationStamp: context.definingUnit.file.modificationStamp, + filePath: currentUnit.file.path, + modificationStamp: currentUnit.file.modificationStamp, contextRoot: context.libraryElement?.session.analysisContext.contextRoot, - resourceProvider: context.definingUnit.file.provider, + resourceProvider: currentUnit.file.provider, + analysisOptionsLoader: analysisOptionsLoader, + ignoreMatcher: ignoreMatcher, ); registry.addCompilationUnit(this, visitor); diff --git a/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart b/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart deleted file mode 100644 index 11e33731..00000000 --- a/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart +++ /dev/null @@ -1,12 +0,0 @@ -import 'package:analyzer/dart/analysis/context_root.dart'; -import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/path_utils.dart'; - -/// Extension methods for [ContextRoot] to help with file analysis checks. -extension ContextRootExtensions on ContextRoot { - /// Checks if [filePath] is excluded from analysis by this context root. - /// - /// Returns `true` only if the file is within the context root but is - /// explicitly excluded (e.g., via analysis_options.yaml). - bool isFileExcluded(String filePath) => - PathUtils.isWithinOrEqual(root.path, filePath) && !isAnalyzed(filePath); -} diff --git a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart index c39435f5..3ca1ca67 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -5,6 +5,7 @@ import 'package:analyzer/diagnostic/diagnostic.dart'; import 'package:analyzer/file_system/file_system.dart'; import 'package:analyzer/file_system/physical_file_system.dart'; import 'package:collection/collection.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/body_candidate.dart'; @@ -12,13 +13,13 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/models/cross_file_mat import 'package:solid_lints/src/lints/avoid_duplicate_code/models/duplicate_location.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/services/global_hash_registry.dart'; -import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/range_extension.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/token_utils.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/descendant_visitor.dart'; import 'package:solid_lints/src/models/solid_diagnostic_message.dart'; +import 'package:solid_lints/src/utils/ignore_matcher.dart'; /// A visitor that detects duplicate code blocks (at the function level and/or /// statement block level) within a single compilation unit and across files. @@ -32,6 +33,8 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final int _modificationStamp; final ContextRoot? _contextRoot; final ResourceProvider _resourceProvider; + final AnalysisOptionsLoader? _analysisOptionsLoader; + final IgnoreMatcher _ignoreMatcher; /// Creates a new instance of [AvoidDuplicateCodeVisitor]. AvoidDuplicateCodeVisitor( @@ -39,24 +42,40 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { this._parameters, { required String filePath, required int modificationStamp, + required IgnoreMatcher ignoreMatcher, ContextRoot? contextRoot, ResourceProvider? resourceProvider, + AnalysisOptionsLoader? analysisOptionsLoader, }) : _filePath = filePath, _modificationStamp = modificationStamp, _contextRoot = contextRoot, _resourceProvider = - resourceProvider ?? PhysicalResourceProvider.INSTANCE; + resourceProvider ?? PhysicalResourceProvider.INSTANCE, + _analysisOptionsLoader = analysisOptionsLoader, + _ignoreMatcher = ignoreMatcher; @override void visitCompilationUnit(CompilationUnit node) { if (_filePath.isEmpty) return; + final filePath = _filePath; + GlobalHashRegistry.instance.resourceProvider = _resourceProvider; - if (_tryReportFromCache( - _filePath, - _contextRoot?.root.path ?? _findPackageRoot(_filePath) ?? '', - )) { + final packageRoot = + _contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; + + final isExcluded = + _analysisOptionsLoader?.isFileExcludedForFile(filePath) ?? false; + + if (isExcluded || _ignoreMatcher.isFileIgnored(node)) { + GlobalHashRegistry.instance.removeFile( + filePath, + parameters: _parameters, + packageRoot: packageRoot, + ); return; } + + if (_tryReportFromCache(filePath, packageRoot)) return; final hasher = AstStructuralHashVisitor( ignoreLiterals: _parameters.ignoreLiterals, ignoreIdentifiers: _parameters.ignoreIdentifiers, @@ -67,7 +86,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { node: hasher.computeHash(node), }; final crossFileDuplicatesByHash = _findAndSaveCrossFileMatches( - _filePath, + filePath, candidates .map( (c) => HashEntry( @@ -79,10 +98,10 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { ), ) .toList(), - _contextRoot?.root.path ?? _findPackageRoot(_filePath) ?? '', + packageRoot, ); _groupAndReportDuplicates( - _filePath, + filePath, candidates, candidateHashes, hasher, @@ -90,6 +109,19 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { ); } + List _collectCandidates(CompilationUnit node) { + final collector = CandidateVisitor(_parameters); + node.accept(collector); + return collector.candidates + .whereNot( + (c) => _ignoreMatcher.isCandidateIgnored( + c.node, + c.enclosingDeclaration, + ), + ) + .toList(); + } + bool _tryReportFromCache(String filePath, String packageRoot) { if (packageRoot.isEmpty) return false; @@ -114,7 +146,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { cachedEntries, parameters: _parameters, packageRoot: packageRoot, - isFileExcluded: (p) => _contextRoot?.isFileExcluded(p) ?? false, + isFileExcluded: _isFileExcluded, ); final hashGroups = groupBy(cachedEntries, (entry) => entry.hash); @@ -170,11 +202,8 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { return true; // Cache hit and handled } - List _collectCandidates(CompilationUnit node) { - final collector = CandidateVisitor(_parameters); - node.accept(collector); - return collector.candidates; - } + bool _isFileExcluded(String path) => + _analysisOptionsLoader?.isFileExcludedForFile(path) ?? false; Map> _findAndSaveCrossFileMatches( String filePath, @@ -189,8 +218,9 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { hashEntries, parameters: _parameters, packageRoot: packageRoot, - isFileExcluded: (p) => _contextRoot?.isFileExcluded(p) ?? false, + isFileExcluded: _isFileExcluded, ); + registry.updateFile( filePath, hashEntries, diff --git a/lib/src/lints/avoid_late_keyword/avoid_late_keyword_rule.dart b/lib/src/lints/avoid_late_keyword/avoid_late_keyword_rule.dart index e9552709..04522c54 100644 --- a/lib/src/lints/avoid_late_keyword/avoid_late_keyword_rule.dart +++ b/lib/src/lints/avoid_late_keyword/avoid_late_keyword_rule.dart @@ -14,14 +14,13 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// avoid_late_keyword: -/// allow_initialized: false -/// ignored_types: -/// - AnimationController -/// - ColorTween +/// solid_lints: +/// diagnostics: +/// avoid_late_keyword: +/// allow_initialized: false +/// ignored_types: +/// - AnimationController +/// - ColorTween /// ``` /// /// ### Example diff --git a/lib/src/lints/avoid_late_keyword/models/avoid_late_keyword_parameters.dart b/lib/src/lints/avoid_late_keyword/models/avoid_late_keyword_parameters.dart index 8073b70a..cc52bb94 100644 --- a/lib/src/lints/avoid_late_keyword/models/avoid_late_keyword_parameters.dart +++ b/lib/src/lints/avoid_late_keyword/models/avoid_late_keyword_parameters.dart @@ -13,12 +13,11 @@ class AvoidLateKeywordParameters { /// Example: /// /// ```yaml - /// plugins: - /// solid_lints: - /// diagnostics: - /// avoid_late_keyword: - /// ignored_types: - /// - ColorTween + /// solid_lints: + /// diagnostics: + /// avoid_late_keyword: + /// ignored_types: + /// - ColorTween /// ``` /// /// ```dart 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 70ba36be..076a1808 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 @@ -14,13 +14,12 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// avoid_non_null_assertion: -/// ignored_types: -/// - IMap -/// - BuiltMap +/// solid_lints: +/// diagnostics: +/// avoid_non_null_assertion: +/// ignored_types: +/// - IMap +/// - BuiltMap /// ``` /// /// ### Example diff --git a/lib/src/lints/avoid_non_null_assertion/models/avoid_non_null_assertion_parameters.dart b/lib/src/lints/avoid_non_null_assertion/models/avoid_non_null_assertion_parameters.dart index e10755e4..6f09d64f 100644 --- a/lib/src/lints/avoid_non_null_assertion/models/avoid_non_null_assertion_parameters.dart +++ b/lib/src/lints/avoid_non_null_assertion/models/avoid_non_null_assertion_parameters.dart @@ -6,12 +6,11 @@ class AvoidNonNullAssertionParameters { /// Example: /// /// ```yaml - /// plugins: - /// solid_lints: - /// diagnostics: - /// avoid_non_null_assertion: - /// ignored_types: - /// - IMap + /// solid_lints: + /// diagnostics: + /// avoid_non_null_assertion: + /// ignored_types: + /// - IMap /// ``` /// /// ```dart 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 4d1c1487..3156af82 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 @@ -18,13 +18,12 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// avoid_returning_widgets: -/// exclude: -/// - class_name: MyWidget -/// method_name: buildCustomButton +/// solid_lints: +/// diagnostics: +/// avoid_returning_widgets: +/// exclude: +/// - class_name: MyWidget +/// method_name: buildCustomButton /// ``` /// /// ### Example diff --git a/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart b/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart index 32aef04e..ecdaf9ab 100644 --- a/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart +++ b/lib/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart @@ -14,15 +14,14 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// avoid_unused_parameters: -/// exclude: -/// - class_name: MyClass -/// method_name: myMethod -/// exclude_annotation: -/// - freezed +/// solid_lints: +/// diagnostics: +/// avoid_unused_parameters: +/// exclude: +/// - class_name: MyClass +/// method_name: myMethod +/// exclude_annotation: +/// - freezed /// ``` /// /// {@template solid_lints.avoid_unused_parameters.example} diff --git a/lib/src/lints/avoid_using_api/avoid_using_api_rule.dart b/lib/src/lints/avoid_using_api/avoid_using_api_rule.dart index 2a03131e..60cade17 100644 --- a/lib/src/lints/avoid_using_api/avoid_using_api_rule.dart +++ b/lib/src/lints/avoid_using_api/avoid_using_api_rule.dart @@ -13,20 +13,18 @@ import 'package:solid_lints/src/models/solid_multi_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// avoid_using_api: -/// avoid_using_api: true -/// severity: warning -/// entries: -/// - class_name: LegacyClient -/// source: package:legacy_api/legacy_api.dart -/// reason: 'Use ModernClient instead.' -/// severity: error -/// - identifier: print -/// source: dart:core -/// reason: 'Use logging framework instead.' +/// solid_lints: +/// diagnostics: +/// avoid_using_api: +/// severity: warning +/// entries: +/// - class_name: LegacyClient +/// source: package:legacy_api/legacy_api.dart +/// reason: 'Use ModernClient instead.' +/// severity: error +/// - identifier: print +/// source: dart:core +/// reason: 'Use logging framework instead.' /// ``` class AvoidUsingApiRule extends SolidMultiLintRule { /// This lint name. diff --git a/lib/src/lints/avoid_using_api/models/avoid_using_api_parameters.dart b/lib/src/lints/avoid_using_api/models/avoid_using_api_parameters.dart index 6e9d074c..b8382fef 100644 --- a/lib/src/lints/avoid_using_api/models/avoid_using_api_parameters.dart +++ b/lib/src/lints/avoid_using_api/models/avoid_using_api_parameters.dart @@ -12,17 +12,15 @@ import 'package:yaml/yaml.dart'; /// /// Example: /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// avoid_using_api: -/// avoid_using_api: error -/// entries: -/// - identifier: wait -/// class_name: Future -/// source: dart:async -/// reason: "Future.wait from dart:async isnt allowed" -/// severity: warning +/// solid_lints: +/// diagnostics: +/// avoid_using_api: +/// entries: +/// - identifier: wait +/// class_name: Future +/// source: dart:async +/// reason: "Future.wait from dart:async isnt allowed" +/// severity: warning /// ``` class AvoidUsingApiParameters { /// A list of BannedCodeOption parameters. diff --git a/lib/src/lints/cyclomatic_complexity/cyclomatic_complexity_rule.dart b/lib/src/lints/cyclomatic_complexity/cyclomatic_complexity_rule.dart index 79a6b671..6e64a5a8 100644 --- a/lib/src/lints/cyclomatic_complexity/cyclomatic_complexity_rule.dart +++ b/lib/src/lints/cyclomatic_complexity/cyclomatic_complexity_rule.dart @@ -17,11 +17,10 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// triggering a warning. /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// cyclomatic_complexity: -/// max_complexity: 10 +/// solid_lints: +/// diagnostics: +/// cyclomatic_complexity: +/// max_complexity: 10 /// ``` class CyclomaticComplexityRule extends SolidLintRule { 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 bff7fa5d..68c8ae8f 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 @@ -11,13 +11,12 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// function_lines_of_code: -/// max_lines: 100 -/// exclude: -/// - build +/// solid_lints: +/// diagnostics: +/// function_lines_of_code: +/// max_lines: 100 +/// exclude: +/// - build /// ``` class FunctionLinesOfCodeRule extends SolidLintRule { diff --git a/lib/src/lints/member_ordering/member_ordering_rule.dart b/lib/src/lints/member_ordering/member_ordering_rule.dart index 0c44f6bb..8cc02c8b 100644 --- a/lib/src/lints/member_ordering/member_ordering_rule.dart +++ b/lib/src/lints/member_ordering/member_ordering_rule.dart @@ -45,15 +45,14 @@ import 'package:solid_lints/src/models/solid_multi_lint_rule.dart'; /// Assuming config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// member_ordering: -/// alphabetize: true -/// order: -/// - fields -/// - getters_setters -/// - methods +/// solid_lints: +/// diagnostics: +/// member_ordering: +/// alphabetize: true +/// order: +/// - fields +/// - getters_setters +/// - methods /// ``` /// /// #### BAD: diff --git a/lib/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart b/lib/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart index fd1a1b65..f3a263ec 100644 --- a/lib/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart +++ b/lib/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart @@ -30,16 +30,15 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// Assuming config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// named_parameters_ordering: -/// order: -/// - required -/// - required_super -/// - default -/// - nullable -/// - super +/// solid_lints: +/// diagnostics: +/// named_parameters_ordering: +/// order: +/// - required +/// - required_super +/// - default +/// - nullable +/// - super /// ``` /// /// #### BAD: diff --git a/lib/src/lints/no_empty_block/no_empty_block_rule.dart b/lib/src/lints/no_empty_block/no_empty_block_rule.dart index 0f15e066..7cb49369 100644 --- a/lib/src/lints/no_empty_block/no_empty_block_rule.dart +++ b/lib/src/lints/no_empty_block/no_empty_block_rule.dart @@ -16,15 +16,14 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// no_empty_block: -/// allow_with_comments: true -/// exclude: -/// - method_name: build -/// - class_name: MyClass -/// method_name: build +/// solid_lints: +/// diagnostics: +/// no_empty_block: +/// allow_with_comments: true +/// exclude: +/// - method_name: build +/// - class_name: MyClass +/// method_name: build /// ``` /// /// ### Example diff --git a/lib/src/lints/no_magic_number/no_magic_number_rule.dart b/lib/src/lints/no_magic_number/no_magic_number_rule.dart index 9380fc03..7dd3d79d 100644 --- a/lib/src/lints/no_magic_number/no_magic_number_rule.dart +++ b/lib/src/lints/no_magic_number/no_magic_number_rule.dart @@ -17,12 +17,11 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// no_magic_number: -/// allowed: [12, 42] -/// allowed_in_widget_params: true +/// solid_lints: +/// diagnostics: +/// no_magic_number: +/// allowed: [12, 42] +/// allowed_in_widget_params: true /// ``` /// /// ### Example diff --git a/lib/src/lints/number_of_parameters/number_of_parameters_rule.dart b/lib/src/lints/number_of_parameters/number_of_parameters_rule.dart index 23841906..5d92846d 100644 --- a/lib/src/lints/number_of_parameters/number_of_parameters_rule.dart +++ b/lib/src/lints/number_of_parameters/number_of_parameters_rule.dart @@ -13,11 +13,10 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// Assuming config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// number_of_parameters: -/// max_parameters: 2 +/// solid_lints: +/// diagnostics: +/// number_of_parameters: +/// max_parameters: 2 /// ``` /// /// #### BAD: diff --git a/lib/src/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart b/lib/src/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart index 4e7731f5..75372939 100644 --- a/lib/src/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart +++ b/lib/src/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart @@ -15,11 +15,10 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// prefer_conditional_expressions: -/// ignore_nested: true +/// solid_lints: +/// diagnostics: +/// prefer_conditional_expressions: +/// ignore_nested: true /// ``` /// /// ### Example diff --git a/lib/src/lints/prefer_match_file_name/prefer_match_file_name_rule.dart b/lib/src/lints/prefer_match_file_name/prefer_match_file_name_rule.dart index d78556d6..ab88d47e 100644 --- a/lib/src/lints/prefer_match_file_name/prefer_match_file_name_rule.dart +++ b/lib/src/lints/prefer_match_file_name/prefer_match_file_name_rule.dart @@ -12,15 +12,14 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// prefer_match_file_name: -/// exclude_entity: -/// - mixin -/// - extension -/// - extension_type -/// - enum +/// solid_lints: +/// diagnostics: +/// prefer_match_file_name: +/// exclude_entity: +/// - mixin +/// - extension +/// - extension_type +/// - enum /// ``` /// /// ## Tests diff --git a/lib/src/lints/use_descriptive_names_for_type_parameters/use_descriptive_names_for_type_parameters_rule.dart b/lib/src/lints/use_descriptive_names_for_type_parameters/use_descriptive_names_for_type_parameters_rule.dart index a994b2b8..3ef9ac3a 100644 --- a/lib/src/lints/use_descriptive_names_for_type_parameters/use_descriptive_names_for_type_parameters_rule.dart +++ b/lib/src/lints/use_descriptive_names_for_type_parameters/use_descriptive_names_for_type_parameters_rule.dart @@ -14,11 +14,10 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// Assuming config: /// /// ```yaml -/// plugins: -/// solid_lints: -/// diagnostics: -/// use_descriptive_names_for_type_parameters: -/// min_type_parameters: 3 +/// solid_lints: +/// diagnostics: +/// use_descriptive_names_for_type_parameters: +/// min_type_parameters: 3 /// ``` /// /// #### BAD: diff --git a/lib/src/models/filtering_diagnostic_reporter.dart b/lib/src/models/filtering_diagnostic_reporter.dart new file mode 100644 index 00000000..bcc1f74f --- /dev/null +++ b/lib/src/models/filtering_diagnostic_reporter.dart @@ -0,0 +1,92 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/diagnostic/diagnostic.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; + +/// A [DiagnosticReporter] decorator that suppresses diagnostic reporting +/// for AST nodes located in files excluded by analysis options. +class FilteringDiagnosticReporter extends DiagnosticReporter { + final DiagnosticReporter _delegate; + final AnalysisOptionsLoader _loader; + + /// Creates a new instance of [FilteringDiagnosticReporter]. + FilteringDiagnosticReporter(this._delegate, this._loader) + : super(DiagnosticListener.nullListener, _delegate.source); + + @override + Diagnostic atNode( + AstNode node, + DiagnosticCode diagnosticCode, { + List? arguments, + List? contextMessages, + }) { + final filePath = switch (node.root) { + CompilationUnit(:final declaredFragment?) => declaredFragment.source, + _ => _delegate.source, + }.fullName; + + if (_loader.isFileExcludedForFile(filePath)) { + return _suppressedDiagnostic(diagnosticCode, node.offset, node.length); + } + + return _delegate.atNode( + node, + diagnosticCode, + arguments: arguments, + contextMessages: contextMessages, + ); + } + + @override + Diagnostic atOffset({ + required int offset, + required int length, + required DiagnosticCode diagnosticCode, + List? arguments, + List? contextMessages, + }) { + if (_loader.isFileExcludedForFile(_delegate.source.fullName)) { + return _suppressedDiagnostic(diagnosticCode, offset, length); + } + + return _delegate.atOffset( + offset: offset, + length: length, + diagnosticCode: diagnosticCode, + arguments: arguments, + contextMessages: contextMessages, + ); + } + + @override + Diagnostic atToken( + Token token, + DiagnosticCode diagnosticCode, { + List? arguments, + List? contextMessages, + }) { + if (_loader.isFileExcludedForFile(_delegate.source.fullName)) { + return _suppressedDiagnostic(diagnosticCode, token.offset, token.length); + } + + return _delegate.atToken( + token, + diagnosticCode, + arguments: arguments, + contextMessages: contextMessages, + ); + } + + Diagnostic _suppressedDiagnostic( + DiagnosticCode diagnosticCode, + int offset, + int length, + ) => Diagnostic.tmp( + source: _delegate.source, + offset: offset, + length: length, + diagnosticCode: diagnosticCode, + ); +} diff --git a/lib/src/models/proxy_analysis_rule.dart b/lib/src/models/proxy_analysis_rule.dart index 47376dd1..831383b7 100644 --- a/lib/src/models/proxy_analysis_rule.dart +++ b/lib/src/models/proxy_analysis_rule.dart @@ -5,6 +5,7 @@ import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; import 'package:analyzer/error/error.dart'; import 'package:analyzer/error/listener.dart'; import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; +import 'package:solid_lints/src/models/filtering_diagnostic_reporter.dart'; /// A proxy wrapper for [AnalysisRule] that checks if the rule is disabled /// before registering its node processors. @@ -38,7 +39,7 @@ class ProxyAnalysisRule extends AnalysisRule { @override set reporter(DiagnosticReporter value) { super.reporter = value; - delegate.reporter = value; + delegate.reporter = FilteringDiagnosticReporter(value, loader); } @override @@ -49,6 +50,9 @@ class ProxyAnalysisRule extends AnalysisRule { if (loader.isRuleDisabled(context, name)) { return; } + if (loader.isFileExcluded(context)) { + return; + } delegate.registerNodeProcessors(registry, context); } } diff --git a/lib/src/models/proxy_multi_analysis_rule.dart b/lib/src/models/proxy_multi_analysis_rule.dart index 5f25dddb..822ecd77 100644 --- a/lib/src/models/proxy_multi_analysis_rule.dart +++ b/lib/src/models/proxy_multi_analysis_rule.dart @@ -5,6 +5,7 @@ import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; import 'package:analyzer/error/error.dart'; import 'package:analyzer/error/listener.dart'; import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; +import 'package:solid_lints/src/models/filtering_diagnostic_reporter.dart'; /// A proxy wrapper for [MultiAnalysisRule] that checks if the rule is disabled /// before registering its node processors. @@ -38,7 +39,7 @@ class ProxyMultiAnalysisRule extends MultiAnalysisRule { @override set reporter(DiagnosticReporter value) { super.reporter = value; - delegate.reporter = value; + delegate.reporter = FilteringDiagnosticReporter(value, loader); } @override @@ -49,6 +50,9 @@ class ProxyMultiAnalysisRule extends MultiAnalysisRule { if (loader.isRuleDisabled(context, name)) { return; } + if (loader.isFileExcluded(context)) { + return; + } delegate.registerNodeProcessors(registry, context); } } diff --git a/lib/src/models/solid_lint_rule.dart b/lib/src/models/solid_lint_rule.dart index 2b705805..d6a5ab7b 100644 --- a/lib/src/models/solid_lint_rule.dart +++ b/lib/src/models/solid_lint_rule.dart @@ -6,7 +6,9 @@ import 'package:solid_lints/src/models/rule_parameters_parser.dart'; /// A base class for emitting information about /// issues with user's `.dart` files. abstract class SolidLintRule extends AnalysisRule { - final AnalysisOptionsLoader? _analysisOptionsLoader; + /// The loader used to read analysis options, rule parameters, and check + /// file exclusions. + final AnalysisOptionsLoader? analysisOptionsLoader; final RuleParametersParser? _parametersParser; @@ -15,24 +17,23 @@ abstract class SolidLintRule extends AnalysisRule { required super.name, required super.description, super.state, - }) : _analysisOptionsLoader = null, + }) : analysisOptionsLoader = null, _parametersParser = null; /// Constructor for [SolidLintRule] model with parameters. SolidLintRule.withParameters({ - required AnalysisOptionsLoader analysisOptionsLoader, + required this.analysisOptionsLoader, required RuleParametersParser parametersParser, required super.name, required super.description, super.state, - }) : _analysisOptionsLoader = analysisOptionsLoader, - _parametersParser = parametersParser; + }) : _parametersParser = parametersParser; /// Reads the rule parameters from analysis options and parses them to [T] T? getParametersForContext(RuleContext context) { - _analysisOptionsLoader?.loadRulesOptionsFromContext(context); + analysisOptionsLoader?.loadRulesOptionsFromContext(context); - final unparsedParameters = _analysisOptionsLoader?.getRuleOptions( + final unparsedParameters = analysisOptionsLoader?.getRuleOptions( context, name, ); diff --git a/lib/src/models/solid_multi_lint_rule.dart b/lib/src/models/solid_multi_lint_rule.dart index 947a4915..00750db1 100644 --- a/lib/src/models/solid_multi_lint_rule.dart +++ b/lib/src/models/solid_multi_lint_rule.dart @@ -2,35 +2,38 @@ 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'; import 'package:solid_lints/src/models/rule_parameters_parser.dart'; +import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// A base class for lint rules that report multiple diagnostic codes /// and require configuration parameters from analysis options. /// -/// Mirrors SolidLintRule but extends MultiAnalysisRule instead of -/// AnalysisRule, allowing rules to define multiple diagnostic codes. +/// Mirrors [SolidLintRule] but extends [MultiAnalysisRule] instead of +/// [AnalysisRule], allowing rules to define multiple diagnostic codes. abstract class SolidMultiLintRule extends MultiAnalysisRule { - final AnalysisOptionsLoader _analysisOptionsLoader; + /// The loader used to read analysis options, rule parameters, and check + /// file exclusions. + final AnalysisOptionsLoader analysisOptionsLoader; final RuleParametersParser _parametersParser; /// Constructor for [SolidMultiLintRule] model with parameters. SolidMultiLintRule({ - required AnalysisOptionsLoader analysisOptionsLoader, + required this.analysisOptionsLoader, required RuleParametersParser parametersParser, required super.name, required super.description, super.state, - }) : _analysisOptionsLoader = analysisOptionsLoader, - _parametersParser = parametersParser; + }) : _parametersParser = parametersParser; /// Reads the rule parameters from analysis options and parses them to [T]. T? getParametersForContext(RuleContext context) { - _analysisOptionsLoader.loadRulesOptionsFromContext(context); + analysisOptionsLoader.loadRulesOptionsFromContext(context); - final unparsedParameters = _analysisOptionsLoader.getRuleOptions( + final unparsedParameters = analysisOptionsLoader.getRuleOptions( context, name, ); + if (unparsedParameters == null) return null; return _parametersParser(unparsedParameters); diff --git a/lib/src/utils/ignore_matcher.dart b/lib/src/utils/ignore_matcher.dart new file mode 100644 index 00000000..123f26b6 --- /dev/null +++ b/lib/src/utils/ignore_matcher.dart @@ -0,0 +1,36 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:solid_lints/src/utils/token_utils.dart'; + +/// Utility class for detecting `// ignore:` and `// ignore_for_file:` comments +/// targeting a specific lint rule. +final class IgnoreMatcher { + /// The name of the rule being checked. + final String ruleName; + + late final _fileIgnoreRegex = RegExp( + '//\\s*ignore_for_file:.*?\\b$ruleName\\b', + caseSensitive: false, + ); + late final _lineIgnoreRegex = RegExp( + '//\\s*ignore:.*?\\b$ruleName\\b', + caseSensitive: false, + ); + + /// Creates a new instance of [IgnoreMatcher] for [ruleName]. + IgnoreMatcher(this.ruleName); + + /// Checks if the entire [unit] is ignored for [ruleName]. + bool isFileIgnored(CompilationUnit unit) => [ + unit.beginToken, + ...unit.directives.map((d) => d.beginToken), + ...unit.declarations.map((d) => d.beginToken), + unit.endToken, + ].commentLexemes.any(_fileIgnoreRegex.hasMatch); + + /// Checks if a candidate [node] or its enclosing [declaration] is ignored. + bool isCandidateIgnored(AstNode node, [Declaration? declaration]) => [ + ?declaration?.beginToken, + ?declaration?.firstTokenAfterCommentAndMetadata, + node.beginToken, + ].commentLexemes.any(_lineIgnoreRegex.hasMatch); +} diff --git a/lib/src/utils/token_utils.dart b/lib/src/utils/token_utils.dart new file mode 100644 index 00000000..2bdd3494 --- /dev/null +++ b/lib/src/utils/token_utils.dart @@ -0,0 +1,19 @@ +import 'package:analyzer/dart/ast/token.dart'; + +/// Extension methods for [Token] manipulation. +extension TokenUtils on Token { + /// Returns an iterable sequence of all preceding comment tokens before + /// this token. + Iterable get comments sync* { + for (var c = precedingComments; c != null; c = c.next as CommentToken?) { + yield c; + } + } +} + +/// Extension methods for [Iterable] manipulation. +extension TokenIterableUtils on Iterable { + /// Returns all comment lexemes from the tokens in this iterable. + Iterable get commentLexemes => + expand((t) => t.comments).map((c) => c.lexeme); +} diff --git a/test/src/common/parameter_parser/analysis_options_loader_test.dart b/test/src/common/parameter_parser/analysis_options_loader_test.dart index 6261eb31..aa404ae3 100644 --- a/test/src/common/parameter_parser/analysis_options_loader_test.dart +++ b/test/src/common/parameter_parser/analysis_options_loader_test.dart @@ -551,9 +551,246 @@ solid_lints: }); } + void test_isFileExcluded_when_matching_glob_pattern() { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +analyzer: + exclude: + - "**/*.g.dart" + - "**/*.freezed.dart" +'''); + + final gDartContext = _createMockContextForFile( + '$testPackageLibPath/models/user.g.dart', + ); + final freezedContext = _createMockContextForFile( + '$testPackageLibPath/models/user.freezed.dart', + ); + final dartContext = _createMockContextForFile( + '$testPackageLibPath/models/user.dart', + ); + + expect(analysisOptionsLoader.isFileExcluded(gDartContext), isTrue); + expect(analysisOptionsLoader.isFileExcluded(freezedContext), isTrue); + expect(analysisOptionsLoader.isFileExcluded(dartContext), isFalse); + } + + void test_isFileExcluded_for_part_file_when_defining_unit_is_not_excluded() { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +analyzer: + exclude: + - "**/*.g.dart" +'''); + + final parentFile = getFile('$testPackageLibPath/models/user.dart'); + final partFile = getFile('$testPackageLibPath/models/user.g.dart'); + final rootFolder = getFolder(testPackageRootPath); + + final partContext = _TestRuleContext( + _TestWorkspacePackage(rootFolder), + definingUnit: _TestRuleContextUnit(parentFile), + currentUnit: _TestRuleContextUnit(partFile), + ); + + final parentContext = _TestRuleContext( + _TestWorkspacePackage(rootFolder), + definingUnit: _TestRuleContextUnit(parentFile), + currentUnit: _TestRuleContextUnit(parentFile), + ); + + expect(analysisOptionsLoader.isFileExcluded(partContext), isTrue); + expect(analysisOptionsLoader.isFileExcluded(parentContext), isFalse); + } + + void test_isFileExcluded_when_matching_exact_path() { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +analyzer: + exclude: + - "lib/generated_plugin_registrant.dart" +'''); + + final registrantContext = _createMockContextForFile( + '$testPackageLibPath/generated_plugin_registrant.dart', + ); + final otherContext = _createMockContextForFile( + '$testPackageLibPath/other.dart', + ); + + expect(analysisOptionsLoader.isFileExcluded(registrantContext), isTrue); + expect(analysisOptionsLoader.isFileExcluded(otherContext), isFalse); + } + + void test_isFileExcluded_inherited_from_included_options() { + final includedOptionsPath = '$testPackageRootPath/included_options.yaml'; + newFile(includedOptionsPath, ''' +analyzer: + exclude: + - "**/*.g.dart" +'''); + + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +include: included_options.yaml +'''); + + final gDartContext = _createMockContextForFile( + '$testPackageLibPath/user.g.dart', + ); + final dartContext = _createMockContextForFile( + '$testPackageLibPath/user.dart', + ); + + expect(analysisOptionsLoader.isFileExcluded(gDartContext), isTrue); + expect(analysisOptionsLoader.isFileExcluded(dartContext), isFalse); + } + + void test_isFileExcluded_merged_with_local_options() { + final includedOptionsPath = '$testPackageRootPath/included_options.yaml'; + newFile(includedOptionsPath, ''' +analyzer: + exclude: + - "**/*.g.dart" +'''); + + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +include: included_options.yaml +analyzer: + exclude: + - "**/*.freezed.dart" +'''); + + final gDartContext = _createMockContextForFile( + '$testPackageLibPath/user.g.dart', + ); + final freezedContext = _createMockContextForFile( + '$testPackageLibPath/user.freezed.dart', + ); + final dartContext = _createMockContextForFile( + '$testPackageLibPath/user.dart', + ); + + expect(analysisOptionsLoader.isFileExcluded(gDartContext), isTrue); + expect(analysisOptionsLoader.isFileExcluded(freezedContext), isTrue); + expect(analysisOptionsLoader.isFileExcluded(dartContext), isFalse); + } + + void test_isFileExcluded_resolves_nearest_nested_analysis_options() { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +analyzer: + exclude: + - "**/*.root_gen.dart" +'''); + + final nestedDirPath = '$testPackageRootPath/nested_pkg'; + newFolder(nestedDirPath); + newAnalysisOptionsYamlFile(nestedDirPath, ''' +analyzer: + exclude: + - "**/*.nested_gen.dart" +'''); + + final rootGenInRoot = _createMockContextForFile( + '$testPackageLibPath/a.root_gen.dart', + ); + final nestedGenInRoot = _createMockContextForFile( + '$testPackageLibPath/a.nested_gen.dart', + ); + final nestedGenInNested = _createMockContextForFile( + '$nestedDirPath/lib/b.nested_gen.dart', + packageRootPath: nestedDirPath, + ); + final rootGenInNested = _createMockContextForFile( + '$nestedDirPath/lib/b.root_gen.dart', + packageRootPath: nestedDirPath, + ); + + expect(analysisOptionsLoader.isFileExcluded(rootGenInRoot), isTrue); + expect(analysisOptionsLoader.isFileExcluded(nestedGenInRoot), isFalse); + expect(analysisOptionsLoader.isFileExcluded(nestedGenInNested), isTrue); + expect(analysisOptionsLoader.isFileExcluded(rootGenInNested), isFalse); + } + + void test_isFileExcludedForFile_matches_path_without_context() { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +analyzer: + exclude: + - "**/*.g.dart" +'''); + + expect( + analysisOptionsLoader.isFileExcludedForFile( + '$testPackageLibPath/user.g.dart', + ), + isTrue, + ); + expect( + analysisOptionsLoader.isFileExcludedForFile( + '$testPackageLibPath/user.dart', + ), + isFalse, + ); + } + + void test_isFileExcludedForFile_returns_false_for_relative_path() { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +analyzer: + exclude: + - "**/*.g.dart" +'''); + + expect( + analysisOptionsLoader.isFileExcludedForFile('lib/user.g.dart'), + isFalse, + ); + } + + void test_isFileExcludedForFile_returns_false_when_no_analysis_options() { + expect( + analysisOptionsLoader.isFileExcludedForFile( + '/some/unrelated/path/user.g.dart', + ), + isFalse, + ); + } + + void test_isFileExcluded_ignores_malformed_glob_patterns() { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +analyzer: + exclude: + - "" + - "[abc" + - "{foo,bar" + - "invalid(glob" + - "**/*.g.dart" +'''); + + final gDartContext = _createMockContextForFile( + '$testPackageLibPath/models/user.g.dart', + ); + final dartContext = _createMockContextForFile( + '$testPackageLibPath/models/user.dart', + ); + + expect(analysisOptionsLoader.isFileExcluded(gDartContext), isTrue); + expect(analysisOptionsLoader.isFileExcluded(dartContext), isFalse); + } + + RuleContext _createMockContextForFile( + String filePath, { + String? packageRootPath, + }) { + final file = getFile(filePath); + final rootFolder = getFolder(packageRootPath ?? testPackageRootPath); + final unit = _TestRuleContextUnit(file); + return _TestRuleContext( + _TestWorkspacePackage(rootFolder), + definingUnit: unit, + currentUnit: unit, + ); + } + RuleContext _createMockContextForPackage( String packageRootPath, { RuleContextUnit? definingUnit, + RuleContextUnit? currentUnit, }) { final rootFolder = getFolder(packageRootPath); return _TestRuleContext( @@ -563,6 +800,7 @@ solid_lints: _TestRuleContextUnit( rootFolder.getChildAssumingFile('lib/dummy.dart'), ), + currentUnit: currentUnit, ); } } @@ -574,7 +812,14 @@ class _TestRuleContext implements RuleContext { @override final RuleContextUnit definingUnit; - _TestRuleContext(this.package, {required this.definingUnit}); + @override + final RuleContextUnit? currentUnit; + + _TestRuleContext( + this.package, { + required this.definingUnit, + this.currentUnit, + }); @override dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); diff --git a/test/src/common/utils/ignore_matcher_test.dart b/test/src/common/utils/ignore_matcher_test.dart new file mode 100644 index 00000000..40e03df3 --- /dev/null +++ b/test/src/common/utils/ignore_matcher_test.dart @@ -0,0 +1,284 @@ +import 'package:analyzer/dart/analysis/utilities.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:solid_lints/src/utils/ignore_matcher.dart'; +import 'package:test/test.dart'; + +void main() { + group('IgnoreMatcher', () { + const ruleName = 'my_rule'; + final matcher = IgnoreMatcher(ruleName); + + group('isFileIgnored', () { + test('returns true for simple ignore_for_file comment', () { + final result = parseString( + content: + ''' +// ignore_for_file: $ruleName + +void foo() {} +''', + ); + + expect(matcher.isFileIgnored(result.unit), isTrue); + }); + + test('returns true for package-prefixed ignore_for_file comment', () { + final result = parseString( + content: + ''' +// ignore_for_file: solid_lints/$ruleName + +void foo() {} +''', + ); + + expect(matcher.isFileIgnored(result.unit), isTrue); + }); + + test('returns true when combined with other ignored rules', () { + final result = parseString( + content: + ''' +// ignore_for_file: other_rule, $ruleName, another_rule + +void foo() {} +''', + ); + + expect(matcher.isFileIgnored(result.unit), isTrue); + }); + + test( + 'returns true when combined with other rules and package prefix', + () { + final result = parseString( + content: + ''' +// ignore_for_file: other_rule, solid_lints/$ruleName, another_rule + +void foo() {} +''', + ); + + expect(matcher.isFileIgnored(result.unit), isTrue); + }, + ); + + test('returns true when placed between directives', () { + final result = parseString( + content: + ''' +import 'dart:async'; + +// ignore_for_file: $ruleName +import 'dart:io'; + +void foo() {} +''', + ); + + expect(matcher.isFileIgnored(result.unit), isTrue); + }); + + test( + 'returns true when placed after directives and before declarations', + () { + final result = parseString( + content: + ''' +import 'dart:async'; + +// ignore_for_file: $ruleName +void foo() {} +''', + ); + + expect(matcher.isFileIgnored(result.unit), isTrue); + }, + ); + + test('returns true when placed at the end of the file', () { + final result = parseString( + content: + ''' +void foo() {} + +// ignore_for_file: $ruleName +''', + ); + + expect(matcher.isFileIgnored(result.unit), isTrue); + }); + + test('returns false when no ignore comments are present', () { + final result = parseString( + content: ''' +void foo() {} +''', + ); + + expect(matcher.isFileIgnored(result.unit), isFalse); + }); + + test('returns false for unrelated rule ignore_for_file comment', () { + final result = parseString( + content: ''' +// ignore_for_file: other_rule + +void foo() {} +''', + ); + + expect(matcher.isFileIgnored(result.unit), isFalse); + }); + + test('works with different custom rule names', () { + const customRule = 'another_custom_rule'; + final customMatcher = IgnoreMatcher(customRule); + final result = parseString( + content: + ''' +// ignore_for_file: $customRule + +void foo() {} +''', + ); + + expect(customMatcher.isFileIgnored(result.unit), isTrue); + expect(matcher.isFileIgnored(result.unit), isFalse); + }); + }); + + group('isCandidateIgnored', () { + test('returns true for inline ignore on function declaration', () { + final (body, decl) = _parseFunction(''' +// ignore: $ruleName +void foo() { + final x = 1; +} +'''); + + expect(matcher.isCandidateIgnored(body, decl), isTrue); + }); + + test('returns true for package-prefixed inline ignore', () { + final (body, decl) = _parseFunction(''' +// ignore: solid_lints/$ruleName +void foo() { + final x = 1; +} +'''); + + expect(matcher.isCandidateIgnored(body, decl), isTrue); + }); + + test( + 'returns true when combined with other rules and package prefix', + () { + final (body, decl) = _parseFunction(''' +// ignore: other_rule, solid_lints/$ruleName, another_rule +void foo() { + final x = 1; +} +'''); + + expect(matcher.isCandidateIgnored(body, decl), isTrue); + }, + ); + + test( + 'returns true when comments explain ignores across multiple lines', + () { + final (body, decl) = _parseFunction(''' +// $ruleName is ignored because reasons +// ignore: $ruleName +// other_rule is ignored because reasons +// ignore: other_rule +void foo() { + final x = 1; +} +'''); + + expect(matcher.isCandidateIgnored(body, decl), isTrue); + }, + ); + + test( + 'returns true when inline ignore is placed after metadata annotation', + () { + final result = parseString( + content: + ''' +class Foo { + @override + // ignore: $ruleName + void foo() { + final x = 1; + } +} +''', + ); + + final classDecl = result.unit.declarations.first as ClassDeclaration; + final methodDecl = switch (classDecl.body) { + BlockClassBody(:final members) => + members.first as MethodDeclaration, + _ => throw StateError('Expected BlockClassBody'), + }; + final body = (methodDecl.body as BlockFunctionBody).block; + + expect(matcher.isCandidateIgnored(body, methodDecl), isTrue); + }, + ); + + test('returns true for inline ignore directly on statement block', () { + final result = parseString( + content: + ''' +void foo() { + // ignore: $ruleName + { + final x = 1; + } +} +''', + ); + + final decl = result.unit.declarations.first as FunctionDeclaration; + final outerBody = + (decl.functionExpression.body as BlockFunctionBody).block; + final innerBlock = outerBody.statements.first as Block; + + expect(matcher.isCandidateIgnored(innerBlock, null), isTrue); + }); + + test('returns false when declaration is not ignored', () { + final (body, decl) = _parseFunction(''' +void foo() { + final x = 1; +} +'''); + + expect(matcher.isCandidateIgnored(body, decl), isFalse); + }); + + test('returns false when another unrelated rule is ignored', () { + final (body, decl) = _parseFunction(''' +// ignore: other_rule +void foo() { + final x = 1; +} +'''); + + expect(matcher.isCandidateIgnored(body, decl), isFalse); + }); + }); + }); +} + +(Block body, FunctionDeclaration decl) _parseFunction(String content) { + final unit = parseString(content: content).unit; + final decl = unit.declarations.first as FunctionDeclaration; + final body = (decl.functionExpression.body as BlockFunctionBody).block; + return (body, decl); +} diff --git a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart index 04573387..68c9c850 100644 --- a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart +++ b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart @@ -1,3 +1,5 @@ +import 'package:analyzer/dart/analysis/utilities.dart' show parseString; +import 'package:analyzer/file_system/file_system.dart'; 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'; @@ -7,6 +9,7 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/avoid_duplicate_code_ import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/services/global_hash_registry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart'; +import 'package:test/test.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; import '../../utils/auto_test_lint_offsets.dart'; @@ -354,24 +357,7 @@ void otherMethod() { } '''); - final resolvedOther = await resolveFile(otherFile.path); - final visitor = AvoidDuplicateCodeVisitor( - rule as AvoidDuplicateCodeRule, - AvoidDuplicateCodeParameters( - minTokens: 15, - ignoreLiterals: false, - ignoreIdentifiers: true, - checkBlocks: true, - exclude: ExcludedIdentifiersListParameter( - exclude: [ExcludedIdentifierParameter(methodName: 'excluded')], - ), - ), - filePath: otherFile.path, - modificationStamp: 1, - contextRoot: resolvedOther.session.analysisContext.contextRoot, - resourceProvider: resourceProvider, - ); - resolvedOther.unit.accept(visitor); + await _indexFile(otherFile); await assertAutoDiagnostics(''' void mainMethod() ${expectLint(r'''{ @@ -482,4 +468,193 @@ void second() { } '''); } + + Future + test_duplicate_code_with_three_files_and_excluded_part_file() async { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +linter: + rules: + - ${rule.name} +analyzer: + exclude: + - "**/*.g.dart" +$_mockAnalysisOptionsContent +'''); + rule = AvoidDuplicateCodeRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + ); + + // 1. First file: other.dart (separate file with identical code) + final otherFile = newFile('$testPackageLibPath/other.dart', ''' +void otherMethod() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} +'''); + + // 2. Second file: part.g.dart (part file with identical code) + newFile('$testPackageLibPath/part.g.dart', ''' +part of 'test.dart'; + +void partMethod() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} +'''); + + // Populate registry with other.dart + await _indexFile(otherFile); + + // 3. Third file: test.dart (main library, not excluded) + // Expect: mainMethod in test.dart is flagged as duplicate of other.dart. + // part.g.dart will NOT trigger diagnostics because .g.dart is excluded. + await assertAutoDiagnostics(''' +part 'part.g.dart'; + +void mainMethod() ${expectLint(r'''{ + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +}''')} +'''); + } + + Future + test_ignored_method_with_comment_does_not_trigger_cross_file_duplicates() async { + // 1. Create other.dart where the method is marked with ignore comment + final otherFile = newFile('$testPackageLibPath/other.dart', ''' +// ignore: solid_lints/avoid_duplicate_code +void ignoredMethod() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} +'''); + + // Populate registry with other.dart + await _indexFile(otherFile); + + // 2. test.dart has identical code, but other.dart's method was ignored. + // Therefore, test.dart should have NO duplicates reported! + await assertNoDiagnostics(''' +void mainMethod() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} +'''); + } + + Future + test_ignored_file_with_ignore_for_file_does_not_trigger_cross_file_duplicates() async { + // 1. Create other.dart with ignore_for_file comment + final otherFile = newFile('$testPackageLibPath/other.dart', ''' +// ignore_for_file: avoid_duplicate_code + +void otherMethod() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} +'''); + + // Populate registry with other.dart + await _indexFile(otherFile); + + // 2. test.dart has identical code, but other.dart was ignored for file. + // Therefore, test.dart should have NO duplicates reported! + await assertNoDiagnostics(''' +void mainMethod() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} +'''); + } + + Future test_visiting_ignored_file_removes_it_from_registry() async { + final otherFile = newFile('$testPackageLibPath/other.dart', ''' +void otherMethod() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} +'''); + await _indexFile(otherFile); + expect(GlobalHashRegistry.instance.fileCount, 1); + + final result = parseString( + content: ''' +// ignore_for_file: avoid_duplicate_code +void otherMethod() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} +''', + ); + final avoidRule = rule as AvoidDuplicateCodeRule; + final visitor = AvoidDuplicateCodeVisitor( + avoidRule, + AvoidDuplicateCodeParameters.empty(), + filePath: otherFile.path, + modificationStamp: 2, + ignoreMatcher: avoidRule.ignoreMatcher, + resourceProvider: resourceProvider, + analysisOptionsLoader: avoidRule.analysisOptionsLoader, + ); + result.unit.accept(visitor); + + expect(GlobalHashRegistry.instance.fileCount, 0); + } + + Future _indexFile( + File file, { + AvoidDuplicateCodeParameters? parameters, + }) async { + final resolved = await resolveFile(file.path); + final avoidRule = rule as AvoidDuplicateCodeRule; + final visitor = AvoidDuplicateCodeVisitor( + avoidRule, + parameters ?? + AvoidDuplicateCodeParameters( + minTokens: 15, + ignoreLiterals: false, + ignoreIdentifiers: true, + checkBlocks: true, + exclude: ExcludedIdentifiersListParameter( + exclude: [ExcludedIdentifierParameter(methodName: 'excluded')], + ), + ), + filePath: file.path, + modificationStamp: 1, + ignoreMatcher: avoidRule.ignoreMatcher, + contextRoot: resolved.session.analysisContext.contextRoot, + resourceProvider: resourceProvider, + analysisOptionsLoader: avoidRule.analysisOptionsLoader, + ); + resolved.unit.accept(visitor); + } } diff --git a/test/src/models/fakes/fake_rule_context.dart b/test/src/models/fakes/fake_rule_context.dart new file mode 100644 index 00000000..94c02132 --- /dev/null +++ b/test/src/models/fakes/fake_rule_context.dart @@ -0,0 +1,7 @@ +import 'package:analyzer/analysis_rule/rule_context.dart'; + +/// A fake implementation of [RuleContext] for model tests. +class FakeRuleContext implements RuleContext { + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} diff --git a/test/src/models/fakes/fake_rule_visitor_registry.dart b/test/src/models/fakes/fake_rule_visitor_registry.dart new file mode 100644 index 00000000..d17e93b0 --- /dev/null +++ b/test/src/models/fakes/fake_rule_visitor_registry.dart @@ -0,0 +1,7 @@ +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; + +/// A fake implementation of [RuleVisitorRegistry] for model tests. +class FakeRuleVisitorRegistry implements RuleVisitorRegistry { + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} diff --git a/test/src/models/fakes/fake_source.dart b/test/src/models/fakes/fake_source.dart new file mode 100644 index 00000000..69b0dbed --- /dev/null +++ b/test/src/models/fakes/fake_source.dart @@ -0,0 +1,13 @@ +import 'package:analyzer/source/source.dart'; + +/// A fake implementation of [Source] for model tests. +class FakeSource implements Source { + @override + final String fullName; + + /// Creates a new instance of [FakeSource]. + FakeSource(this.fullName); + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} diff --git a/test/src/models/filtering_diagnostic_reporter_test.dart b/test/src/models/filtering_diagnostic_reporter_test.dart new file mode 100644 index 00000000..2e19398d --- /dev/null +++ b/test/src/models/filtering_diagnostic_reporter_test.dart @@ -0,0 +1,146 @@ +import 'package:analyzer/diagnostic/diagnostic.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:analyzer_testing/src/analysis_rule/pub_package_resolution.dart'; +import 'package:solid_lints/src/models/filtering_diagnostic_reporter.dart'; +import 'package:test/test.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../utils/fake_analysis_options_loader.dart'; +import 'fakes/fake_source.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(FilteringDiagnosticReporterTest); + }); +} + +@reflectiveTest +class FilteringDiagnosticReporterTest extends PubPackageResolutionTest { + static const _testCode = LintCode('test_code', 'Test problem'); + + late _RecordingDiagnosticListener listener; + late DiagnosticReporter delegate; + late Map excludedFiles; + late FakeAnalysisOptionsLoader loader; + late FilteringDiagnosticReporter reporter; + + String get mainFilePath => '$testPackageLibPath/main.dart'; + String get partFilePath => '$testPackageLibPath/part.g.dart'; + + @override + void setUp() { + super.setUp(); + listener = _RecordingDiagnosticListener(); + delegate = DiagnosticReporter(listener, FakeSource(mainFilePath)); + excludedFiles = {}; + loader = FakeAnalysisOptionsLoader(excludedFiles: excludedFiles); + reporter = FilteringDiagnosticReporter(delegate, loader); + } + + Future test_atNode_forwards_when_file_is_not_excluded() async { + newFile(mainFilePath, 'int x = 1;'); + final resolved = await resolveFile(mainFilePath); + final node = resolved.unit.declarations.first; + + excludedFiles[mainFilePath] = false; + + reporter.atNode(node, _testCode); + + expect(listener.diagnostics, hasLength(1)); + expect(listener.diagnostics.first.diagnosticCode, equals(_testCode)); + } + + Future test_atNode_suppresses_when_main_file_is_excluded() async { + newFile(mainFilePath, 'int x = 1;'); + final resolved = await resolveFile(mainFilePath); + final node = resolved.unit.declarations.first; + + excludedFiles[mainFilePath] = true; + + reporter.atNode(node, _testCode); + + expect(listener.diagnostics, isEmpty); + } + + Future + test_atNode_suppresses_when_part_file_is_excluded_and_main_is_not() async { + newFile(partFilePath, ''' +part of 'main.dart'; +int partVar = 2; +'''); + newFile(mainFilePath, ''' +part 'part.g.dart'; +int mainVar = 1; +'''); + + final partResolved = await resolveFile(partFilePath); + final partNode = partResolved.unit.declarations.first; + + final mainResolved = await resolveFile(mainFilePath); + final mainNode = mainResolved.unit.declarations.first; + + excludedFiles[mainFilePath] = false; + excludedFiles[partFilePath] = true; + + // Node from part file should be suppressed + reporter.atNode(partNode, _testCode); + expect(listener.diagnostics, isEmpty); + + // Node from main file should still be reported + reporter.atNode(mainNode, _testCode); + expect(listener.diagnostics, hasLength(1)); + expect(listener.diagnostics.first.diagnosticCode, equals(_testCode)); + } + + Future test_atOffset_forwards_when_file_is_not_excluded() async { + excludedFiles[mainFilePath] = false; + + reporter.atOffset(offset: 0, length: 10, diagnosticCode: _testCode); + + expect(listener.diagnostics, hasLength(1)); + expect(listener.diagnostics.first.diagnosticCode, equals(_testCode)); + } + + Future test_atOffset_suppresses_when_file_is_excluded() async { + excludedFiles[mainFilePath] = true; + + reporter.atOffset(offset: 0, length: 10, diagnosticCode: _testCode); + + expect(listener.diagnostics, isEmpty); + } + + Future test_atToken_forwards_when_file_is_not_excluded() async { + newFile(mainFilePath, 'int x = 1;'); + final resolved = await resolveFile(mainFilePath); + final token = resolved.unit.beginToken; + + excludedFiles[mainFilePath] = false; + + reporter.atToken(token, _testCode); + + expect(listener.diagnostics, hasLength(1)); + expect(listener.diagnostics.first.diagnosticCode, equals(_testCode)); + } + + Future test_atToken_suppresses_when_file_is_excluded() async { + newFile(mainFilePath, 'int x = 1;'); + final resolved = await resolveFile(mainFilePath); + final token = resolved.unit.beginToken; + + excludedFiles[mainFilePath] = true; + + reporter.atToken(token, _testCode); + + expect(listener.diagnostics, isEmpty); + } +} + +class _RecordingDiagnosticListener implements DiagnosticListener { + final List diagnostics = []; + + @override + void onDiagnostic(Diagnostic diagnostic) { + diagnostics.add(diagnostic); + } +} diff --git a/test/src/models/proxy_analysis_rule_test.dart b/test/src/models/proxy_analysis_rule_test.dart new file mode 100644 index 00000000..b31fa3c5 --- /dev/null +++ b/test/src/models/proxy_analysis_rule_test.dart @@ -0,0 +1,111 @@ +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:analyzer/error/listener.dart'; +import 'package:solid_lints/src/models/filtering_diagnostic_reporter.dart'; +import 'package:solid_lints/src/models/proxy_analysis_rule.dart'; +import 'package:test/test.dart'; + +import '../utils/fake_analysis_options_loader.dart'; +import 'fakes/fake_rule_context.dart'; +import 'fakes/fake_rule_visitor_registry.dart'; +import 'fakes/fake_source.dart'; + +void main() { + group('ProxyAnalysisRule', () { + late _MockAnalysisRule delegate; + late FakeAnalysisOptionsLoader loader; + late FakeRuleVisitorRegistry registry; + late FakeRuleContext context; + late ProxyAnalysisRule proxyRule; + + setUp(() { + delegate = _MockAnalysisRule(); + loader = FakeAnalysisOptionsLoader(); + registry = FakeRuleVisitorRegistry(); + context = FakeRuleContext(); + proxyRule = ProxyAnalysisRule(delegate, loader); + }); + + test('delegates to wrapped rule when enabled and not excluded', () { + loader.isRuleDisabledResult = false; + loader.isFileExcludedResult = false; + + proxyRule.registerNodeProcessors(registry, context); + + expect(delegate.registerNodeProcessorsCalled, isTrue); + expect(delegate.lastRegistry, same(registry)); + expect(delegate.lastContext, same(context)); + }); + + test('does not delegate when rule is disabled', () { + loader.isRuleDisabledResult = true; + loader.isFileExcludedResult = false; + + proxyRule.registerNodeProcessors(registry, context); + + expect(delegate.registerNodeProcessorsCalled, isFalse); + }); + + test('does not delegate when file is excluded', () { + loader.isRuleDisabledResult = false; + loader.isFileExcludedResult = true; + + proxyRule.registerNodeProcessors(registry, context); + + expect(delegate.registerNodeProcessorsCalled, isFalse); + }); + + test('wraps reporter in FilteringDiagnosticReporter', () { + final originalReporter = DiagnosticReporter( + DiagnosticListener.nullListener, + FakeSource('/path/to/file.dart'), + ); + + proxyRule.reporter = originalReporter; + + expect(delegate.reporter, isA()); + }); + + test('delegates getters and properties', () { + expect(proxyRule.diagnosticCode, equals(delegate.diagnosticCode)); + expect(proxyRule.name, equals(delegate.name)); + expect(proxyRule.description, equals(delegate.description)); + expect(proxyRule.canUseParsedResult, equals(delegate.canUseParsedResult)); + expect(proxyRule.incompatibleRules, equals(delegate.incompatibleRules)); + expect(proxyRule.pubspecVisitor, equals(delegate.pubspecVisitor)); + }); + }); +} + +class _MockAnalysisRule extends AnalysisRule { + bool registerNodeProcessorsCalled = false; + RuleVisitorRegistry? lastRegistry; + RuleContext? lastContext; + DiagnosticReporter? lastReporter; + + _MockAnalysisRule() + : super(name: 'mock_rule', description: 'Mock rule description'); + + @override + DiagnosticCode get diagnosticCode => const LintCode('mock_rule', 'Mock code'); + + DiagnosticReporter get reporter => lastReporter!; + + @override + set reporter(DiagnosticReporter value) { + super.reporter = value; + lastReporter = value; + } + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + registerNodeProcessorsCalled = true; + lastRegistry = registry; + lastContext = context; + } +} diff --git a/test/src/models/proxy_multi_analysis_rule_test.dart b/test/src/models/proxy_multi_analysis_rule_test.dart new file mode 100644 index 00000000..17bb8b66 --- /dev/null +++ b/test/src/models/proxy_multi_analysis_rule_test.dart @@ -0,0 +1,117 @@ +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:analyzer/error/listener.dart'; +import 'package:solid_lints/src/models/filtering_diagnostic_reporter.dart'; +import 'package:solid_lints/src/models/proxy_multi_analysis_rule.dart'; +import 'package:test/test.dart'; + +import '../utils/fake_analysis_options_loader.dart'; +import 'fakes/fake_rule_context.dart'; +import 'fakes/fake_rule_visitor_registry.dart'; +import 'fakes/fake_source.dart'; + +void main() { + group('ProxyMultiAnalysisRule', () { + late _MockMultiAnalysisRule delegate; + late FakeAnalysisOptionsLoader loader; + late FakeRuleVisitorRegistry registry; + late FakeRuleContext context; + late ProxyMultiAnalysisRule proxyRule; + + setUp(() { + delegate = _MockMultiAnalysisRule(); + loader = FakeAnalysisOptionsLoader(); + registry = FakeRuleVisitorRegistry(); + context = FakeRuleContext(); + proxyRule = ProxyMultiAnalysisRule(delegate, loader); + }); + + test('delegates to wrapped rule when enabled and not excluded', () { + loader.isRuleDisabledResult = false; + loader.isFileExcludedResult = false; + + proxyRule.registerNodeProcessors(registry, context); + + expect(delegate.registerNodeProcessorsCalled, isTrue); + expect(delegate.lastRegistry, same(registry)); + expect(delegate.lastContext, same(context)); + }); + + test('does not delegate when rule is disabled', () { + loader.isRuleDisabledResult = true; + loader.isFileExcludedResult = false; + + proxyRule.registerNodeProcessors(registry, context); + + expect(delegate.registerNodeProcessorsCalled, isFalse); + }); + + test('does not delegate when file is excluded', () { + loader.isRuleDisabledResult = false; + loader.isFileExcludedResult = true; + + proxyRule.registerNodeProcessors(registry, context); + + expect(delegate.registerNodeProcessorsCalled, isFalse); + }); + + test('wraps reporter in FilteringDiagnosticReporter', () { + final originalReporter = DiagnosticReporter( + DiagnosticListener.nullListener, + FakeSource('/path/to/file.dart'), + ); + + proxyRule.reporter = originalReporter; + + expect(delegate.reporter, isA()); + }); + + test('delegates getters and properties', () { + expect(proxyRule.diagnosticCodes, equals(delegate.diagnosticCodes)); + expect(proxyRule.name, equals(delegate.name)); + expect(proxyRule.description, equals(delegate.description)); + expect(proxyRule.canUseParsedResult, equals(delegate.canUseParsedResult)); + expect(proxyRule.incompatibleRules, equals(delegate.incompatibleRules)); + expect(proxyRule.pubspecVisitor, equals(delegate.pubspecVisitor)); + }); + }); +} + +class _MockMultiAnalysisRule extends MultiAnalysisRule { + bool registerNodeProcessorsCalled = false; + RuleVisitorRegistry? lastRegistry; + RuleContext? lastContext; + DiagnosticReporter? lastReporter; + + _MockMultiAnalysisRule() + : super( + name: 'mock_multi_rule', + description: 'Mock multi rule description', + ); + + @override + List get diagnosticCodes => const [ + LintCode('mock_multi_rule_1', 'Mock code 1'), + LintCode('mock_multi_rule_2', 'Mock code 2'), + ]; + + DiagnosticReporter get reporter => lastReporter!; + + @override + set reporter(DiagnosticReporter value) { + super.reporter = value; + lastReporter = value; + } + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + registerNodeProcessorsCalled = true; + lastRegistry = registry; + lastContext = context; + } +} diff --git a/test/src/utils/fake_analysis_options_loader.dart b/test/src/utils/fake_analysis_options_loader.dart index c3e16e2a..5909b297 100644 --- a/test/src/utils/fake_analysis_options_loader.dart +++ b/test/src/utils/fake_analysis_options_loader.dart @@ -1,10 +1,32 @@ import 'package:analyzer/analysis_rule/rule_context.dart'; import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; +/// A fake implementation of [AnalysisOptionsLoader] for testing. class FakeAnalysisOptionsLoader implements AnalysisOptionsLoader { + /// Options for rules returned by [getRuleOptions] and + /// [getRuleOptionsForFile]. final Map ruleOptions; - FakeAnalysisOptionsLoader({required this.ruleOptions}); + /// Excluded files for [isFileExcludedForFile]. + final Map excludedFiles; + + /// Result returned by [isRuleDisabled]. + bool isRuleDisabledResult; + + /// Result returned by [isFileExcluded]. + bool isFileExcludedResult; + + /// Result returned by [isFileExcludedForFile]. + bool? isFileExcludedForFileResult; + + /// Creates a new instance of [FakeAnalysisOptionsLoader]. + FakeAnalysisOptionsLoader({ + this.ruleOptions = const {}, + this.excludedFiles = const {}, + this.isRuleDisabledResult = false, + this.isFileExcludedResult = false, + this.isFileExcludedForFileResult, + }); @override Map? getRuleOptions(RuleContext context, String ruleName) => @@ -20,5 +42,15 @@ class FakeAnalysisOptionsLoader implements AnalysisOptionsLoader { void loadRulesOptionsFromContext(RuleContext context) {} @override - bool isRuleDisabled(RuleContext context, String ruleName) => false; + bool isRuleDisabled(RuleContext context, String ruleName) => + isRuleDisabledResult; + + @override + bool isFileExcluded(RuleContext context) => isFileExcludedResult; + + @override + bool isFileExcludedForFile(String filePath) => + excludedFiles[filePath] ?? + isFileExcludedForFileResult ?? + isFileExcludedResult; }