-
Notifications
You must be signed in to change notification settings - Fork 83
feat: add support for configuration error severities in analysis_options.yaml
#326
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
base: main
Are you sure you want to change the base?
Changes from 10 commits
ae42d24
53f2a59
3c11288
1595a84
2fba63c
3f3a6a1
4f11ecc
135070a
374e367
c398b9d
04b7f96
774d137
314d5db
981698a
bd5ecbf
5f9d12e
cc61df7
1908992
f03e181
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 |
---|---|---|
@@ -0,0 +1,148 @@ | ||
import 'dart:convert'; | ||
import 'dart:io'; | ||
|
||
import 'package:analyzer/error/error.dart'; | ||
import 'package:test/test.dart'; | ||
|
||
import 'cli_process_test.dart'; | ||
import 'create_project.dart'; | ||
import 'peer_project_meta.dart'; | ||
|
||
void main() { | ||
group('Errors severities override', () { | ||
Future<ProcessResult> runProcess(String workingDirectoryPath) async => | ||
Process.run( | ||
'dart', | ||
[customLintBinPath], | ||
workingDirectory: workingDirectoryPath, | ||
stdoutEncoding: utf8, | ||
stderrEncoding: utf8, | ||
); | ||
|
||
Directory createLintUsageWith({ | ||
required Uri pluginUri, | ||
required String analysisOptions, | ||
}) => | ||
createLintUsage( | ||
name: 'test_app', | ||
source: {'lib/main.dart': 'void fn() {}'}, | ||
plugins: {'test_lint': pluginUri}, | ||
analysisOptions: analysisOptions, | ||
); | ||
|
||
Directory createTestPlugin({ | ||
ErrorSeverity errorSeverity = ErrorSeverity.INFO, | ||
}) => | ||
createPlugin( | ||
name: 'test_lint', | ||
main: createPluginSource([ | ||
TestLintRule( | ||
code: 'test_lint', | ||
message: 'Test lint message', | ||
errorSeverity: errorSeverity, | ||
), | ||
]), | ||
); | ||
test('correctly applies error severity from analysis_options.yaml', | ||
() async { | ||
final plugin = createTestPlugin(errorSeverity: ErrorSeverity.ERROR); | ||
|
||
final app = createLintUsageWith( | ||
pluginUri: plugin.uri, | ||
analysisOptions: ''' | ||
custom_lint: | ||
errors: | ||
test_lint: error | ||
''', | ||
); | ||
|
||
final process = await runProcess(app.path); | ||
|
||
expect(trimDependencyOverridesWarning(process.stderr), isEmpty); | ||
expect(process.stdout, ''' | ||
Analyzing... | ||
|
||
lib/main.dart:1:6 • Test lint message • test_lint • ERROR | ||
|
||
1 issue found. | ||
'''); | ||
expect(process.exitCode, 1); | ||
}); | ||
|
||
test('correctly applies warning severity from analysis_options.yaml', | ||
() async { | ||
final plugin = createTestPlugin(); | ||
|
||
final app = createLintUsageWith( | ||
pluginUri: plugin.uri, | ||
analysisOptions: ''' | ||
custom_lint: | ||
errors: | ||
test_lint: warning | ||
''', | ||
); | ||
|
||
final process = await runProcess(app.path); | ||
|
||
expect(trimDependencyOverridesWarning(process.stderr), isEmpty); | ||
expect(process.stdout, ''' | ||
Analyzing... | ||
|
||
lib/main.dart:1:6 • Test lint message • test_lint • WARNING | ||
|
||
1 issue found. | ||
'''); | ||
expect(process.exitCode, 1); | ||
}); | ||
|
||
test('correctly applies info severity from analysis_options.yaml', | ||
() async { | ||
final plugin = createTestPlugin(); | ||
|
||
final app = createLintUsageWith( | ||
pluginUri: plugin.uri, | ||
analysisOptions: ''' | ||
custom_lint: | ||
errors: | ||
test_lint: info | ||
''', | ||
); | ||
|
||
final process = await runProcess(app.path); | ||
|
||
expect(trimDependencyOverridesWarning(process.stderr), isEmpty); | ||
expect(process.stdout, ''' | ||
Analyzing... | ||
|
||
lib/main.dart:1:6 • Test lint message • test_lint • INFO | ||
|
||
1 issue found. | ||
'''); | ||
expect(process.exitCode, 1); | ||
}); | ||
|
||
test('correctly applies none severity from analysis_options.yaml', | ||
() async { | ||
final plugin = createTestPlugin(); | ||
|
||
final app = createLintUsageWith( | ||
pluginUri: plugin.uri, | ||
analysisOptions: ''' | ||
custom_lint: | ||
errors: | ||
test_lint: none | ||
''', | ||
); | ||
|
||
final process = await runProcess(app.path); | ||
|
||
expect(trimDependencyOverridesWarning(process.stderr), isEmpty); | ||
expect(process.stdout, ''' | ||
Analyzing... | ||
|
||
No issues found! | ||
'''); | ||
expect(process.exitCode, 0); | ||
}); | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -72,23 +72,28 @@ class CustomAnalyzerConverter { | |||||
/// start column. If an analysis [options] is provided then the severities of | ||||||
/// the errors will be altered based on those options. | ||||||
List<plugin.AnalysisError> convertAnalysisErrors( | ||||||
List<analyzer.AnalysisError> errors, | ||||||
{analyzer.LineInfo? lineInfo, | ||||||
analyzer.AnalysisOptions? options}) { | ||||||
var serverErrors = <plugin.AnalysisError>[]; | ||||||
for (var error in errors) { | ||||||
var processor = analyzer.ErrorProcessor.getProcessor(options, error); | ||||||
if (processor != null) { | ||||||
var severity = processor.severity; | ||||||
// Errors with null severity are filtered out. | ||||||
if (severity != null) { | ||||||
// Specified severities override. | ||||||
serverErrors.add(convertAnalysisError(error, | ||||||
lineInfo: lineInfo, severity: severity)); | ||||||
} | ||||||
} else { | ||||||
serverErrors.add(convertAnalysisError(error, lineInfo: lineInfo)); | ||||||
List<analyzer.AnalysisError> errors, { | ||||||
|
CustomAnalyzerConverter().convertAnalysisErrors( | |
allAnalysisErrors, |
Does something like:
...convertAnalysisError(...).map((error) => error.copyWith(error: <apply error config>))
Then the file stays untouched, and the location of the logic is in a more sensible place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed it. It was the hardest part in this batch of changes. So please take a close look at this. I've moved logic from custom_analyzer_converter.dart to client.dart
.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import 'package:analyzer/error/error.dart'; | ||
import 'package:analyzer/file_system/file_system.dart'; | ||
import 'package:collection/collection.dart'; | ||
import 'package:meta/meta.dart'; | ||
|
@@ -17,6 +18,7 @@ class CustomLintConfigs { | |
required this.verbose, | ||
required this.debug, | ||
required this.rules, | ||
required this.errors, | ||
}); | ||
|
||
/// Decode a [CustomLintConfigs] from a file. | ||
|
@@ -108,11 +110,33 @@ class CustomLintConfigs { | |
} | ||
} | ||
|
||
final errors = <String, ErrorSeverity>{...includedOptions.errors}; | ||
|
||
final errorsYaml = customLint['errors'] as Object?; | ||
if (errorsYaml is Map) { | ||
for (final entry in errorsYaml.entries) { | ||
final value = entry.value; | ||
if (entry.key case final String key?) { | ||
final severity = ErrorSeverity.values.firstWhereOrNull( | ||
(e) => e.displayName == value, | ||
|
||
); | ||
if (severity == null) { | ||
throw ArgumentError( | ||
'Provided error severity: $value specified for key: $key is not valid. ' | ||
'Valid error severities are: ${ErrorSeverity.values.map((e) => e.displayName).join(', ')}', | ||
); | ||
} | ||
errors[key] = severity; | ||
} | ||
} | ||
} | ||
|
||
return CustomLintConfigs( | ||
enableAllLintRules: enableAllLintRules, | ||
verbose: verbose, | ||
debug: debug, | ||
rules: UnmodifiableMapView(rules), | ||
errors: UnmodifiableMapView(errors), | ||
); | ||
} | ||
|
||
|
@@ -123,6 +147,7 @@ class CustomLintConfigs { | |
verbose: false, | ||
debug: false, | ||
rules: {}, | ||
errors: {}, | ||
); | ||
|
||
/// A field representing whether to enable/disable lint rules that are not | ||
|
@@ -147,20 +172,26 @@ class CustomLintConfigs { | |
/// Whether enable hot-reload and log the VM-service URI. | ||
final bool debug; | ||
|
||
/// A map of lint rules to their severity. This is used to override the severity | ||
/// of a lint rule for a specific lint. | ||
final Map<String, ErrorSeverity> errors; | ||
|
||
@override | ||
bool operator ==(Object other) => | ||
other is CustomLintConfigs && | ||
other.enableAllLintRules == enableAllLintRules && | ||
other.verbose == verbose && | ||
other.debug == debug && | ||
const MapEquality<String, LintOptions>().equals(other.rules, rules); | ||
const MapEquality<String, LintOptions>().equals(other.rules, rules) && | ||
const MapEquality<String, ErrorSeverity>().equals(other.errors, errors); | ||
|
||
@override | ||
int get hashCode => Object.hash( | ||
enableAllLintRules, | ||
verbose, | ||
debug, | ||
const MapEquality<String, LintOptions>().hash(rules), | ||
const MapEquality<String, ErrorSeverity>().hash(errors), | ||
); | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.