From 3cd67d6f3b6d829baaa343208cfb2ef6a153ed5c Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Thu, 2 Jul 2026 20:10:58 +0300 Subject: [PATCH 1/4] fix: resolve rules configuration being ignored in nested directories --- lib/analysis_options.yaml | 58 +++++++-------- lib/analysis_options_test.yaml | 12 ++-- .../analysis_options_loader.dart | 46 ++++++------ .../analysis_options_loader_test.dart | 71 ++++++++++++++++--- 4 files changed, 122 insertions(+), 65 deletions(-) diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index 21857abc..f615faea 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -35,34 +35,34 @@ analyzer: parameter_assignments: error unnecessary_cast: warning -custom_lint: - rules: - - avoid_global_state +solid_lints: + diagnostics: + avoid_global_state: true - - avoid_late_keyword: + avoid_late_keyword: allow_initialized: true ignored_types: - AnimationController - - avoid_non_null_assertion - - avoid_returning_widgets - - avoid_unnecessary_return_variable - - avoid_unnecessary_setstate - - avoid_unnecessary_type_assertions - - avoid_unrelated_type_assertions - - avoid_unused_parameters - - avoid_debug_print_in_release - - avoid_final_with_getter + avoid_non_null_assertion: true + avoid_returning_widgets: true + avoid_unnecessary_return_variable: true + avoid_unnecessary_setstate: true + avoid_unnecessary_type_assertions: true + avoid_unrelated_type_assertions: true + avoid_unused_parameters: true + avoid_debug_print_in_release: true + avoid_final_with_getter: true - - cyclomatic_complexity: + cyclomatic_complexity: max_complexity: 10 - - double_literal_format + double_literal_format: true - - function_lines_of_code: + function_lines_of_code: max_lines: 200 - - member_ordering: + member_ordering: order: - fields - getters_setters @@ -76,26 +76,28 @@ custom_lint: - deactivate_method - dispose_method - - newline_before_return - - no_empty_block - - no_equal_then_else - - prefer_early_return + newline_before_return: true + no_empty_block: true + no_equal_then_else: true + prefer_early_return: true - - no_magic_number: + no_magic_number: allowed_in_widget_params: true - - number_of_parameters: + number_of_parameters: max_parameters: 7 exclude: - method_name: copyWith - - prefer_conditional_expressions: + prefer_conditional_expressions: ignore_nested: true - - prefer_first - - prefer_last - - prefer_match_file_name - - proper_super_calls + prefer_first: true + prefer_last: true + prefer_match_file_name: true + proper_super_calls: true + named_parameters_ordering: true + use_nearest_context: true linter: rules: diff --git a/lib/analysis_options_test.yaml b/lib/analysis_options_test.yaml index 5db9e6b7..19ddb7ec 100644 --- a/lib/analysis_options_test.yaml +++ b/lib/analysis_options_test.yaml @@ -1,18 +1,18 @@ include: package:solid_lints/analysis_options.yaml -custom_lint: - rules: +solid_lints: + diagnostics: # Tests usually organized in one large main() function making this rule not applicable. # Given the quite large threshold configured for this metric we considered extracting # test body into separate function, but that means that we'll have to either pass # Test Context that contains all defined variables in main to every function call # or moving them to the Global State. # Both options didn't look right, so we decided that tests are ok to be long. - - function_lines_of_code: false + function_lines_of_code: false # Since we're not using the source-lines-of-code rule, `main()` function in test can # have high cyclomatic complexity. # For rationale against splitting up `main()` in tests, see `source-lines-of-code` comments. - - cyclomatic_complexity: false + cyclomatic_complexity: false # Late keyword is allowed in tests in order to enable the use of custom mocks and # fakes. @@ -48,6 +48,6 @@ custom_lint: # - In terms of behavior similar to `late`, but requires using the operator in # many places inside the test code, adding uninformative visual noise. # - The use of this operator is also discouraged by the main rule set. - - avoid_late_keyword: false + avoid_late_keyword: false # It's acceptable to include stubs or other helper classes into the test file. - - prefer_match_file_name: false + prefer_match_file_name: false diff --git a/lib/src/common/parameter_parser/analysis_options_loader.dart b/lib/src/common/parameter_parser/analysis_options_loader.dart index f4fb74e8..dfea075a 100644 --- a/lib/src/common/parameter_parser/analysis_options_loader.dart +++ b/lib/src/common/parameter_parser/analysis_options_loader.dart @@ -4,6 +4,8 @@ import 'package:analyzer/file_system/physical_file_system.dart'; import 'package:solid_lints/src/common/parameter_parser/cached_package_rules.dart'; import 'package:yaml/yaml.dart'; +typedef _OptionsMap = Map; + /// Loads and parses analysis options from a Dart project's YAML file. class AnalysisOptionsLoader { final ResourceProvider _resourceProvider; @@ -11,8 +13,7 @@ class AnalysisOptionsLoader { /// Creates an instance of [AnalysisOptionsLoader] AnalysisOptionsLoader({ResourceProvider? resourceProvider}) - : _resourceProvider = - resourceProvider ?? PhysicalResourceProvider.INSTANCE; + : _resourceProvider = resourceProvider ?? PhysicalResourceProvider.INSTANCE; /// Gets the options for a specific rule by its name. Map? getRuleOptions(RuleContext context, String ruleName) => @@ -49,10 +50,10 @@ class AnalysisOptionsLoader { RuleContext context, T Function(String) f, ) { - final packageRootPath = context.package?.root.path; - if (packageRootPath == null) return null; + final filePath = context.definingUnit.file.path; + final dirPath = _resourceProvider.pathContext.dirname(filePath); + final yamlPath = _findNearestAnalysisOptionsFilePath(dirPath); - final yamlPath = _findNearestAnalysisOptionsFilePath(packageRootPath); if (yamlPath == null) return null; return f(yamlPath); @@ -79,8 +80,10 @@ class AnalysisOptionsLoader { String currentDirectoryPath = packageRootPath; while (pathContext.dirname(currentDirectoryPath) != currentDirectoryPath) { - final candidatePath = - pathContext.join(currentDirectoryPath, 'analysis_options.yaml'); + final candidatePath = pathContext.join( + currentDirectoryPath, + 'analysis_options.yaml', + ); final candidateFile = _resourceProvider.getFile(candidatePath); if (candidateFile.exists) { @@ -102,24 +105,23 @@ class AnalysisOptionsLoader { final optionsString = analysisOptionsFile.readAsStringSync(); Object? yaml; try { - yaml = loadYaml(optionsString) as Object?; - } catch (err) { + yaml = loadYaml(optionsString); + } catch (_) { return {}; } - if (yaml - case {'plugins': {'solid_lints': {'diagnostics': final diagnostics?}}} - when diagnostics is Map) { - return Map.fromEntries( - diagnostics.entries.where((e) => e.key is String && e.value is Map).map( - (e) => MapEntry( - e.key as String, - Map.from(e.value as Map), - ), - ), - ); - } + final rawDiagnostics = yaml is _OptionsMap + ? ((yaml['solid_lints'] as _OptionsMap?)?['diagnostics'] ?? + ((yaml['plugins'] as _OptionsMap?)?['solid_lints'] + as _OptionsMap?)?['diagnostics']) + : null; + + if (rawDiagnostics is! Map) return {}; - return {}; + return { + for (final entry in rawDiagnostics.entries) + if (entry.key is String && entry.value is Map) + entry.key as String: Map.from(entry.value as Map), + }; } } 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 a727136f..c487f668 100644 --- a/test/src/common/parameter_parser/analysis_options_loader_test.dart +++ b/test/src/common/parameter_parser/analysis_options_loader_test.dart @@ -25,7 +25,7 @@ plugins: solid_lints: diagnostics: $_mockRuleThatNeedsConfigName: - abc: def + some_parameter: root_value $_mockRule2Name: foo: bar exclude: @@ -43,7 +43,7 @@ plugins: solid_lints: diagnostics: $_mockRuleThatNeedsConfigName: - abc: ghi + some_parameter: nested_value $_mockRule2Name: foo: baz exclude: @@ -128,7 +128,7 @@ plugins: ); expect(mockRuleThatNeedsConfigOptions, isNotNull); - expect(mockRuleThatNeedsConfigOptions, {'abc': 'def'}); + expect(mockRuleThatNeedsConfigOptions, {'some_parameter': 'root_value'}); expect(mockRule2Options, isNotNull); expect(mockRule2Options, { @@ -165,8 +165,8 @@ plugins: _mockRuleThatNeedsConfigName, ); - expect(initialOptions, {'abc': 'def'}); - expect(updatedOptions, {'abc': 'ghi'}); + expect(initialOptions, {'some_parameter': 'root_value'}); + expect(updatedOptions, {'some_parameter': 'nested_value'}); expect(updatedOptions, isNot(same(initialOptions))); } @@ -179,7 +179,7 @@ plugins: ); expect(options, isNotNull); - expect(options, {'abc': 'def'}); + expect(options, {'some_parameter': 'root_value'}); } void test_returns_cached_response_for_same_rule_name() { @@ -197,9 +197,46 @@ plugins: expect(secondOptions, same(firstOptions)); } - RuleContext _createMockContextForPackage(String packageRootPath) { + void test_resolves_nested_analysis_options_for_nested_files() { + final nestedDirPath = '$testPackageRootPath/test'; + final nestedFilePath = '$nestedDirPath/some_test.dart'; + + newFolder(nestedDirPath); + newFile(nestedFilePath, 'void main() {}'); + + newAnalysisOptionsYamlFile( + nestedDirPath, + _mockDifferentAnalysisOptionsContent, + ); + + final nestedFile = getFile(nestedFilePath); + final mockNestedContext = _createMockContextForPackage( + testPackageRootPath, + definingUnit: _TestRuleContextUnit(nestedFile), + ); + + analysisOptionsLoader.loadRulesOptionsFromContext(mockNestedContext); + + final options = analysisOptionsLoader.getRuleOptions( + mockNestedContext, + _mockRuleThatNeedsConfigName, + ); + + expect(options, isNotNull); + expect(options, {'some_parameter': 'nested_value'}); + } + + RuleContext _createMockContextForPackage( + String packageRootPath, { + RuleContextUnit? definingUnit, + }) { + final rootFolder = getFolder(packageRootPath); return _TestRuleContext( - _TestWorkspacePackage(getFolder(packageRootPath)), + _TestWorkspacePackage(rootFolder), + definingUnit: definingUnit ?? + _TestRuleContextUnit( + rootFolder.getChildAssumingFile('lib/dummy.dart'), + ), ); } } @@ -208,7 +245,23 @@ class _TestRuleContext implements RuleContext { @override final WorkspacePackage? package; - _TestRuleContext(this.package); + @override + final RuleContextUnit definingUnit; + + _TestRuleContext( + this.package, { + required this.definingUnit, + }); + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +class _TestRuleContextUnit implements RuleContextUnit { + @override + final File file; + + _TestRuleContextUnit(this.file); @override dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); From 89ca842d7cd68dbd89137a9833446f31a0c1ab97 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Thu, 2 Jul 2026 20:22:53 +0300 Subject: [PATCH 2/4] refactor: use pattern matching for yaml parsing to improve robustness and prevent crashes on invalid configurations --- .../analysis_options_loader.dart | 15 +++++---- .../analysis_options_loader_test.dart | 32 ++++++++++++++++++- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/lib/src/common/parameter_parser/analysis_options_loader.dart b/lib/src/common/parameter_parser/analysis_options_loader.dart index dfea075a..016db159 100644 --- a/lib/src/common/parameter_parser/analysis_options_loader.dart +++ b/lib/src/common/parameter_parser/analysis_options_loader.dart @@ -4,8 +4,6 @@ import 'package:analyzer/file_system/physical_file_system.dart'; import 'package:solid_lints/src/common/parameter_parser/cached_package_rules.dart'; import 'package:yaml/yaml.dart'; -typedef _OptionsMap = Map; - /// Loads and parses analysis options from a Dart project's YAML file. class AnalysisOptionsLoader { final ResourceProvider _resourceProvider; @@ -110,11 +108,14 @@ class AnalysisOptionsLoader { return {}; } - final rawDiagnostics = yaml is _OptionsMap - ? ((yaml['solid_lints'] as _OptionsMap?)?['diagnostics'] ?? - ((yaml['plugins'] as _OptionsMap?)?['solid_lints'] - as _OptionsMap?)?['diagnostics']) - : null; + 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 {}; 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 c487f668..1c1850dd 100644 --- a/test/src/common/parameter_parser/analysis_options_loader_test.dart +++ b/test/src/common/parameter_parser/analysis_options_loader_test.dart @@ -179,7 +179,37 @@ plugins: ); expect(options, isNotNull); - expect(options, {'some_parameter': 'root_value'}); + } + + void test_does_not_crash_when_plugins_is_list() { + newAnalysisOptionsYamlFile( + testPackageRootPath, + ''' +plugins: + - solid_lints +''', + ); + analysisOptionsLoader.loadRulesOptionsFromContext(mockRuleContext); + final options = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _mockRuleThatNeedsConfigName, + ); + expect(options, isNull); + } + + void test_does_not_crash_when_solid_lints_is_boolean() { + newAnalysisOptionsYamlFile( + testPackageRootPath, + ''' +solid_lints: true +''', + ); + analysisOptionsLoader.loadRulesOptionsFromContext(mockRuleContext); + final options = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _mockRuleThatNeedsConfigName, + ); + expect(options, isNull); } void test_returns_cached_response_for_same_rule_name() { From e80e227af6d7afea725222ea03da53113549d23a Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Thu, 2 Jul 2026 20:31:10 +0300 Subject: [PATCH 3/4] fix: improve robustness of analysis options loader by filtering non-string keys and fixing infinite loop in path traversal --- .../analysis_options_loader.dart | 15 +++++++++---- .../analysis_options_loader_test.dart | 21 +++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/lib/src/common/parameter_parser/analysis_options_loader.dart b/lib/src/common/parameter_parser/analysis_options_loader.dart index 016db159..0afea791 100644 --- a/lib/src/common/parameter_parser/analysis_options_loader.dart +++ b/lib/src/common/parameter_parser/analysis_options_loader.dart @@ -73,11 +73,11 @@ class AnalysisOptionsLoader { ); } - String? _findNearestAnalysisOptionsFilePath(String packageRootPath) { + String? _findNearestAnalysisOptionsFilePath(String startDirectoryPath) { final pathContext = _resourceProvider.pathContext; - String currentDirectoryPath = packageRootPath; + String currentDirectoryPath = startDirectoryPath; - while (pathContext.dirname(currentDirectoryPath) != currentDirectoryPath) { + while (true) { final candidatePath = pathContext.join( currentDirectoryPath, 'analysis_options.yaml', @@ -89,6 +89,9 @@ class AnalysisOptionsLoader { } final parentDir = pathContext.dirname(currentDirectoryPath); + if (parentDir == currentDirectoryPath) { + break; + } currentDirectoryPath = parentDir; } @@ -122,7 +125,11 @@ class AnalysisOptionsLoader { return { for (final entry in rawDiagnostics.entries) if (entry.key is String && entry.value is Map) - entry.key as String: Map.from(entry.value as Map), + entry.key as String: { + for (final optionEntry in (entry.value as Map).entries) + if (optionEntry.key is String) + optionEntry.key as String: optionEntry.value, + }, }; } } 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 1c1850dd..67fffb6a 100644 --- a/test/src/common/parameter_parser/analysis_options_loader_test.dart +++ b/test/src/common/parameter_parser/analysis_options_loader_test.dart @@ -212,6 +212,27 @@ solid_lints: true expect(options, isNull); } + void test_does_not_crash_when_rule_option_has_non_string_key() { + newAnalysisOptionsYamlFile( + testPackageRootPath, + ''' +plugins: + solid_lints: + diagnostics: + $_mockRuleThatNeedsConfigName: + 123: true + some_parameter: root_value +''', + ); + analysisOptionsLoader.loadRulesOptionsFromContext(mockRuleContext); + final options = analysisOptionsLoader.getRuleOptions( + mockRuleContext, + _mockRuleThatNeedsConfigName, + ); + expect(options, isNotNull); + expect(options, {'some_parameter': 'root_value'}); + } + void test_returns_cached_response_for_same_rule_name() { analysisOptionsLoader.loadRulesOptionsFromContext(mockRuleContext); From d7db9fd7fdd9c9acf8e63ab1dfedeab6a1cba165 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Thu, 2 Jul 2026 20:37:38 +0300 Subject: [PATCH 4/4] refactor: move file reading inside the try-catch block in analysis_options_loader --- lib/src/common/parameter_parser/analysis_options_loader.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/common/parameter_parser/analysis_options_loader.dart b/lib/src/common/parameter_parser/analysis_options_loader.dart index 0afea791..5b0a016e 100644 --- a/lib/src/common/parameter_parser/analysis_options_loader.dart +++ b/lib/src/common/parameter_parser/analysis_options_loader.dart @@ -103,9 +103,9 @@ class AnalysisOptionsLoader { return {}; } - final optionsString = analysisOptionsFile.readAsStringSync(); Object? yaml; try { + final optionsString = analysisOptionsFile.readAsStringSync(); yaml = loadYaml(optionsString); } catch (_) { return {};