-
Notifications
You must be signed in to change notification settings - Fork 24
fix: support package-level analysis options and rule suppression #318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
ed46119
dbdc1d4
930b5bc
bc38e85
ca83d32
6d8fc43
73d9e8b
716ffda
865d78e
fb53e8b
450acf6
f4cdfdf
39f1ee9
eb9e57c
d87e4ba
75b98a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,4 @@ | ||
| analyzer: | ||
| plugins: | ||
| - custom_lint | ||
| exclude: | ||
| # General generated files | ||
| - "**/*.g.dart" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| /// Shared constants for the solid_lints package. | ||
| const kPluginName = 'solid_lints'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be better if we made it into a static class instead, and hungarian notation is kinda redundant too
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,17 +1,32 @@ | ||||||
| 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/analysis_options_parser.dart'; | ||||||
| import 'package:solid_lints/src/common/parameter_parser/cached_package_rules.dart'; | ||||||
| import 'package:yaml/yaml.dart'; | ||||||
| import 'package:solid_lints/src/common/parameter_parser/package_config_resolver.dart'; | ||||||
|
|
||||||
| /// Loads and parses analysis options from a Dart project's YAML file. | ||||||
| class AnalysisOptionsLoader { | ||||||
| final ResourceProvider _resourceProvider; | ||||||
| final Map<String, CachedPackageRules> _rulesCache = {}; | ||||||
|
|
||||||
| /// Creates an instance of [AnalysisOptionsLoader] | ||||||
| AnalysisOptionsLoader({ResourceProvider? resourceProvider}) | ||||||
| : _resourceProvider = resourceProvider ?? PhysicalResourceProvider.INSTANCE; | ||||||
| // Caches directory path -> nearest analysis_options.yaml file path | ||||||
| final Map<String, String?> _nearestYamlCache = {}; | ||||||
|
|
||||||
| late final AnalysisOptionsParser _parser; | ||||||
|
|
||||||
| /// Creates an instance of [AnalysisOptionsLoader]. | ||||||
| AnalysisOptionsLoader({ | ||||||
| ResourceProvider? resourceProvider, | ||||||
| AnalysisOptionsParser? parser, | ||||||
| }) : _resourceProvider = | ||||||
| resourceProvider ?? PhysicalResourceProvider.INSTANCE { | ||||||
| _parser = parser ?? | ||||||
| AnalysisOptionsParser( | ||||||
| _resourceProvider, | ||||||
| PackageConfigResolver(_resourceProvider), | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| /// Gets the options for a specific rule by its name. | ||||||
| Map<String, Object?>? getRuleOptions(RuleContext context, String ruleName) => | ||||||
|
|
@@ -36,6 +51,17 @@ class AnalysisOptionsLoader { | |||||
| return _rulesCache[yamlPath]?.rules[ruleName]; | ||||||
| } | ||||||
|
|
||||||
| /// Checks if a rule is explicitly disabled. | ||||||
| bool isRuleDisabled(RuleContext context, String ruleName) => | ||||||
| _withNearestAnalysisOptionsFilePathForContext<bool>( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like type can be inferred here
Suggested change
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||
| context, | ||||||
| (path) { | ||||||
| _loadRulesOptionsIfNewer(path); | ||||||
| return _rulesCache[path]?.disabledRules.contains(ruleName) ?? false; | ||||||
| }, | ||||||
| ) ?? | ||||||
| false; | ||||||
|
|
||||||
| /// Loads lint rules from the analysis options file for all rules | ||||||
| /// using the provided [RuleContext]. | ||||||
| void loadRulesOptionsFromContext(RuleContext context) => | ||||||
|
|
@@ -66,70 +92,38 @@ class AnalysisOptionsLoader { | |||||
| return; | ||||||
| } | ||||||
|
|
||||||
| final rules = _getRules(analysisOptionsFile); | ||||||
| final rulesData = _parser.parse(analysisOptionsFile); | ||||||
| _rulesCache[yamlPath] = CachedPackageRules( | ||||||
| modificationStamp: modificationStamp, | ||||||
| rules: rules, | ||||||
| rules: rulesData.rules, | ||||||
| disabledRules: rulesData.disabledRules, | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| String? _findNearestAnalysisOptionsFilePath(String startDirectoryPath) { | ||||||
| final pathContext = _resourceProvider.pathContext; | ||||||
| String currentDirectoryPath = startDirectoryPath; | ||||||
|
|
||||||
| while (true) { | ||||||
| final candidatePath = pathContext.join( | ||||||
| currentDirectoryPath, | ||||||
| 'analysis_options.yaml', | ||||||
| ); | ||||||
| final candidateFile = _resourceProvider.getFile(candidatePath); | ||||||
|
|
||||||
| if (candidateFile.exists) { | ||||||
| return candidatePath; | ||||||
| } | ||||||
|
|
||||||
| final parentDir = pathContext.dirname(currentDirectoryPath); | ||||||
| if (parentDir == currentDirectoryPath) { | ||||||
| break; | ||||||
| return _nearestYamlCache.putIfAbsent(startDirectoryPath, () { | ||||||
| final pathContext = _resourceProvider.pathContext; | ||||||
| var currentDirectoryPath = startDirectoryPath; | ||||||
|
|
||||||
| while (currentDirectoryPath.isNotEmpty) { | ||||||
| final candidatePath = pathContext.join( | ||||||
| currentDirectoryPath, | ||||||
| 'analysis_options.yaml', | ||||||
| ); | ||||||
| final candidateFile = _resourceProvider.getFile(candidatePath); | ||||||
|
|
||||||
| if (candidateFile.exists) { | ||||||
| return candidatePath; | ||||||
| } | ||||||
|
|
||||||
| final parentDir = pathContext.dirname(currentDirectoryPath); | ||||||
| if (parentDir == currentDirectoryPath) { | ||||||
| break; | ||||||
| } | ||||||
| currentDirectoryPath = parentDir; | ||||||
| } | ||||||
| currentDirectoryPath = parentDir; | ||||||
| } | ||||||
|
|
||||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| Map<String, Map<String, Object?>> _getRules(File? analysisOptionsFile) { | ||||||
| if (analysisOptionsFile == null || !analysisOptionsFile.exists) { | ||||||
| return {}; | ||||||
| } | ||||||
|
|
||||||
| Object? yaml; | ||||||
| try { | ||||||
| final optionsString = analysisOptionsFile.readAsStringSync(); | ||||||
| yaml = loadYaml(optionsString); | ||||||
| } catch (_) { | ||||||
| return {}; | ||||||
| } | ||||||
|
|
||||||
| Object? rawDiagnostics; | ||||||
| if (yaml case {'solid_lints': {'diagnostics': final diagnostics}}) { | ||||||
| rawDiagnostics = diagnostics; | ||||||
| } else if (yaml case { | ||||||
| 'plugins': {'solid_lints': {'diagnostics': final diagnostics}}, | ||||||
| }) { | ||||||
| rawDiagnostics = diagnostics; | ||||||
| } | ||||||
|
|
||||||
| if (rawDiagnostics is! Map) return {}; | ||||||
|
|
||||||
| return { | ||||||
| for (final entry in rawDiagnostics.entries) | ||||||
| if (entry.key is String && entry.value is Map) | ||||||
| entry.key as String: <String, Object?>{ | ||||||
| for (final optionEntry in (entry.value as Map).entries) | ||||||
| if (optionEntry.key is String) | ||||||
| optionEntry.key as String: optionEntry.value, | ||||||
| }, | ||||||
| }; | ||||||
| return null; | ||||||
| }); | ||||||
| } | ||||||
|
solid-illiaaihistov marked this conversation as resolved.
|
||||||
| } | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have a lot of cases like: for (final MapEntry(:key, ...) in ...)
if (key is String)
...
we should extract an extension extension on Map<T, K> {
Map<U, K> whereKeyType<U>();
}
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,196 @@ | ||||||||||||||||||
| import 'package:analyzer/file_system/file_system.dart'; | ||||||||||||||||||
| import 'package:solid_lints/src/common/constants.dart'; | ||||||||||||||||||
| import 'package:solid_lints/src/common/parameter_parser/package_config_resolver.dart'; | ||||||||||||||||||
| import 'package:solid_lints/src/common/parameter_parser/rules_data.dart'; | ||||||||||||||||||
| import 'package:yaml/yaml.dart'; | ||||||||||||||||||
|
|
||||||||||||||||||
| /// Parser for analysis_options.yaml files to extract RulesData. | ||||||||||||||||||
| class AnalysisOptionsParser { | ||||||||||||||||||
| final ResourceProvider _resourceProvider; | ||||||||||||||||||
| final PackageConfigResolver _packageConfigResolver; | ||||||||||||||||||
|
|
||||||||||||||||||
| /// Creates a new instance of [AnalysisOptionsParser]. | ||||||||||||||||||
| AnalysisOptionsParser(this._resourceProvider, this._packageConfigResolver); | ||||||||||||||||||
|
|
||||||||||||||||||
| /// Parses the given [analysisOptionsFile] and resolves its imports | ||||||||||||||||||
| /// to return [RulesData]. | ||||||||||||||||||
| RulesData parse(File? analysisOptionsFile) { | ||||||||||||||||||
| return _parseWithSeen(analysisOptionsFile, {}); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| RulesData _parseWithSeen(File? analysisOptionsFile, Set<String> seenPaths) { | ||||||||||||||||||
| if (analysisOptionsFile == null || !analysisOptionsFile.exists) { | ||||||||||||||||||
| return const RulesData.empty(); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| final path = analysisOptionsFile.path; | ||||||||||||||||||
| if (seenPaths.contains(path)) { | ||||||||||||||||||
| return const RulesData.empty(); | ||||||||||||||||||
| } | ||||||||||||||||||
| seenPaths.add(path); | ||||||||||||||||||
|
|
||||||||||||||||||
| final yaml = _parseYaml(analysisOptionsFile); | ||||||||||||||||||
| if (yaml == null) { | ||||||||||||||||||
| return const RulesData.empty(); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| final mergedRules = <String, Map<String, Object?>>{}; | ||||||||||||||||||
| final disabledRules = <String>{}; | ||||||||||||||||||
|
|
||||||||||||||||||
| _resolveAndMergeIncludes( | ||||||||||||||||||
| analysisOptionsFile, | ||||||||||||||||||
| yaml, | ||||||||||||||||||
| seenPaths, | ||||||||||||||||||
| mergedRules, | ||||||||||||||||||
| disabledRules, | ||||||||||||||||||
| ); | ||||||||||||||||||
| _parseRuleOptions(yaml, mergedRules, disabledRules); | ||||||||||||||||||
| _parseSuppressedErrors(yaml, mergedRules, disabledRules); | ||||||||||||||||||
|
|
||||||||||||||||||
| return RulesData(rules: mergedRules, disabledRules: disabledRules); | ||||||||||||||||||
| } | ||||||||||||||||||
|
solid-illiaaihistov marked this conversation as resolved.
|
||||||||||||||||||
|
|
||||||||||||||||||
| Map<dynamic, dynamic>? _parseYaml(File file) { | ||||||||||||||||||
| try { | ||||||||||||||||||
| final optionsString = file.readAsStringSync(); | ||||||||||||||||||
| final parsed = loadYaml(optionsString); | ||||||||||||||||||
| return parsed is Map ? parsed : null; | ||||||||||||||||||
| } catch (_) { | ||||||||||||||||||
| return null; | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
solid-illiaaihistov marked this conversation as resolved.
Outdated
|
||||||||||||||||||
|
|
||||||||||||||||||
| void _resolveAndMergeIncludes( | ||||||||||||||||||
| File baseFile, | ||||||||||||||||||
| Map<dynamic, dynamic> yaml, | ||||||||||||||||||
| Set<String> seenPaths, | ||||||||||||||||||
| Map<String, Map<String, Object?>> mergedRules, | ||||||||||||||||||
| Set<String> disabledRules, | ||||||||||||||||||
| ) { | ||||||||||||||||||
| final includeOption = yaml['include']; | ||||||||||||||||||
| if (includeOption is! String) return; | ||||||||||||||||||
|
|
||||||||||||||||||
| final includedFile = _resolveIncludedFile(baseFile, includeOption); | ||||||||||||||||||
| if (includedFile == null) return; | ||||||||||||||||||
|
|
||||||||||||||||||
| final includedData = _parseWithSeen(includedFile, seenPaths); | ||||||||||||||||||
| mergedRules.addAll(includedData.rules); | ||||||||||||||||||
| disabledRules.addAll(includedData.disabledRules); | ||||||||||||||||||
| } | ||||||||||||||||||
|
solid-illiaaihistov marked this conversation as resolved.
|
||||||||||||||||||
|
|
||||||||||||||||||
| File? _resolveIncludedFile(File baseFile, String includePath) { | ||||||||||||||||||
| final pathContext = _resourceProvider.pathContext; | ||||||||||||||||||
| if (includePath.startsWith('package:')) { | ||||||||||||||||||
| final resolvedPath = _packageConfigResolver.resolvePackageUri( | ||||||||||||||||||
| baseFile.path, | ||||||||||||||||||
| includePath, | ||||||||||||||||||
| ); | ||||||||||||||||||
| if (resolvedPath != null) { | ||||||||||||||||||
| return _resourceProvider.getFile(resolvedPath); | ||||||||||||||||||
| } | ||||||||||||||||||
| return null; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| final baseDir = pathContext.dirname(baseFile.path); | ||||||||||||||||||
| final resolvedPath = pathContext.join(baseDir, includePath); | ||||||||||||||||||
| return _resourceProvider.getFile(resolvedPath); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| void _parseRuleOptions( | ||||||||||||||||||
| Map<dynamic, dynamic> yaml, | ||||||||||||||||||
| Map<String, Map<String, Object?>> mergedRules, | ||||||||||||||||||
| Set<String> disabledRules, | ||||||||||||||||||
| ) { | ||||||||||||||||||
| final rawDiagnostics = _extractDiagnostics(yaml); | ||||||||||||||||||
| if (rawDiagnostics is! Map) return; | ||||||||||||||||||
|
|
||||||||||||||||||
| for (final entry in rawDiagnostics.entries) { | ||||||||||||||||||
| final key = entry.key; | ||||||||||||||||||
| if (key is! String) continue; | ||||||||||||||||||
|
|
||||||||||||||||||
| final ruleName = key; | ||||||||||||||||||
| final value = entry.value; | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||||||||||||||
|
|
||||||||||||||||||
| if (value is Map) { | ||||||||||||||||||
| final existingOptions = mergedRules[ruleName] ?? {}; | ||||||||||||||||||
| mergedRules[ruleName] = <String, Object?>{ | ||||||||||||||||||
| ...existingOptions, | ||||||||||||||||||
| for (final optionEntry in value.entries) | ||||||||||||||||||
| if (optionEntry.key is String) | ||||||||||||||||||
| optionEntry.key as String: optionEntry.value, | ||||||||||||||||||
| }; | ||||||||||||||||||
| disabledRules.remove(ruleName); | ||||||||||||||||||
| } else if (value is bool) { | ||||||||||||||||||
| if (value) { | ||||||||||||||||||
| disabledRules.remove(ruleName); | ||||||||||||||||||
| } else { | ||||||||||||||||||
| mergedRules.remove(ruleName); | ||||||||||||||||||
| disabledRules.add(ruleName); | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
solid-illiaaihistov marked this conversation as resolved.
Outdated
solid-illiaaihistov marked this conversation as resolved.
|
||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| Object? _extractDiagnostics(Map<dynamic, dynamic> yaml) { | ||||||||||||||||||
| final pluginConfig = yaml[kPluginName]; | ||||||||||||||||||
| if (pluginConfig is Map) { | ||||||||||||||||||
| return pluginConfig['diagnostics']; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| final pluginsConfig = yaml['plugins']; | ||||||||||||||||||
| if (pluginsConfig is Map) { | ||||||||||||||||||
| final pluginSubConfig = pluginsConfig[kPluginName]; | ||||||||||||||||||
| if (pluginSubConfig is Map) { | ||||||||||||||||||
| return pluginSubConfig['diagnostics']; | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| return null; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /// Parses rule suppression configured under `analyzer: errors:`. | ||||||||||||||||||
| /// | ||||||||||||||||||
| /// By default, when a user excludes/ignores a rule using the IDE | ||||||||||||||||||
| /// quick fix or standard settings, the Dart analysis server appends | ||||||||||||||||||
| /// the following to `analysis_options.yaml`: | ||||||||||||||||||
| /// | ||||||||||||||||||
| /// ```yaml | ||||||||||||||||||
| /// analyzer: | ||||||||||||||||||
| /// errors: | ||||||||||||||||||
| /// solid_lints/rule_name: ignore | ||||||||||||||||||
| /// ``` | ||||||||||||||||||
| /// | ||||||||||||||||||
| /// To respect this standard mechanism and prevent the plugin rules | ||||||||||||||||||
| /// from running, we parse this section and add suppressed rules | ||||||||||||||||||
| /// to [disabledRules]. | ||||||||||||||||||
| void _parseSuppressedErrors( | ||||||||||||||||||
| Map<dynamic, dynamic> yaml, | ||||||||||||||||||
| Map<String, Map<String, Object?>> mergedRules, | ||||||||||||||||||
| Set<String> disabledRules, | ||||||||||||||||||
| ) { | ||||||||||||||||||
| final analyzer = yaml['analyzer']; | ||||||||||||||||||
| if (analyzer is! Map) return; | ||||||||||||||||||
|
|
||||||||||||||||||
| final errors = analyzer['errors']; | ||||||||||||||||||
| if (errors is! Map) return; | ||||||||||||||||||
|
|
||||||||||||||||||
| const pluginPrefix = '$kPluginName/'; | ||||||||||||||||||
|
|
||||||||||||||||||
| for (final entry in errors.entries) { | ||||||||||||||||||
| final key = entry.key; | ||||||||||||||||||
| if (key is! String) continue; | ||||||||||||||||||
|
|
||||||||||||||||||
| final errorValue = entry.value; | ||||||||||||||||||
| final ruleName = key.startsWith(pluginPrefix) | ||||||||||||||||||
| ? key.substring(pluginPrefix.length) | ||||||||||||||||||
| : key; | ||||||||||||||||||
|
|
||||||||||||||||||
| if (errorValue == 'ignore') { | ||||||||||||||||||
| mergedRules.remove(ruleName); | ||||||||||||||||||
| disabledRules.add(ruleName); | ||||||||||||||||||
| } else { | ||||||||||||||||||
| disabledRules.remove(ruleName); | ||||||||||||||||||
| } | ||||||||||||||||||
|
solid-illiaaihistov marked this conversation as resolved.
Outdated
|
||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
solid-illiaaihistov marked this conversation as resolved.
|
||||||||||||||||||
| } | ||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.