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..5b0a016e 100644 --- a/lib/src/common/parameter_parser/analysis_options_loader.dart +++ b/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? 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,13 +73,15 @@ 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) { @@ -88,6 +89,9 @@ class AnalysisOptionsLoader { } 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.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: { + 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 a727136f..67fffb6a 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,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);