fix: resolve rules configuration being ignored in nested directories

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:

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

lib/src/common/parameter_parser/analysis_options_loader.dart

@@ -11,8 +11,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<String, Object?>? getRuleOptions(RuleContext context, String ruleName) =>
@@ -49,10 +48,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);
@@ -74,20 +73,25 @@ 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) {
-      final candidatePath =
-          pathContext.join(currentDirectoryPath, 'analysis_options.yaml');
+    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;
+      }
       currentDirectoryPath = parentDir;
     }
 
@@ -99,27 +103,33 @@ class AnalysisOptionsLoader {
       return {};
     }
 
-    final optionsString = analysisOptionsFile.readAsStringSync();
     Object? yaml;
     try {
-      yaml = loadYaml(optionsString) as Object?;
-    } catch (err) {
+      final optionsString = analysisOptionsFile.readAsStringSync();
+      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<String, Object?>.from(e.value as Map),
-              ),
-            ),
-      );
+    Object? rawDiagnostics;
+    if (yaml case {'solid_lints': {'diagnostics': final diagnostics}}) {
+      rawDiagnostics = diagnostics;
+    } else if (yaml case {
+      'plugins': {'solid_lints': {'diagnostics': final diagnostics}},
+    }) {
+      rawDiagnostics = diagnostics;
     }
 
-    return {};
+    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,
+          },
+    };
   }
 }

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,58 @@ plugins:
     );
 
     expect(options, isNotNull);
-    expect(options, {'abc': 'def'});
+  }
+
+  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_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() {
@@ -197,9 +248,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 +296,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);