From f0834dbf21ed3a0d64cc081ce546a8726784f842 Mon Sep 17 00:00:00 2001 From: Remi Rousselet Date: Sun, 4 Feb 2024 16:02:11 +0100 Subject: [PATCH] Support --fix and fix-all assist (#223) --- README.md | 4 +- packages/custom_lint/CHANGELOG.md | 4 + packages/custom_lint/bin/custom_lint.dart | 7 + .../lib/custom_lint_example_lint.dart | 2 +- packages/custom_lint/lib/custom_lint.dart | 4 + .../lib/src/analyzer_plugin_starter.dart | 5 +- .../src/v2/custom_lint_analyzer_plugin.dart | 7 + .../lib/src/v2/server_to_client_channel.dart | 1 + packages/custom_lint/lib/src/workspace.dart | 2 +- packages/custom_lint/pubspec.yaml | 1 + .../custom_lint/test/cli_process_test.dart | 127 +++++++ packages/custom_lint/test/cli_test.dart | 4 +- packages/custom_lint/test/create_project.dart | 158 ++++---- packages/custom_lint/test/fixes_test.dart | 347 ++++++++++++++++-- packages/custom_lint/test/goldens.dart | 205 +++++++++-- .../test/goldens/fixes/add_ignore.diff | 74 ++++ .../custom_lint/test/goldens/fixes/fixes.diff | 44 +++ .../test/goldens/fixes/multi_change.diff | 34 ++ .../test/goldens/fixes/no_change.diff | 12 + .../test/goldens/fixes/silenced_change.diff | 24 ++ .../test/goldens/fixes/single_fix.diff | 8 + .../test/goldens/fixes/update_ignore.diff | 25 ++ .../test/goldens/ignore_quick_fix.json | 83 ++++- .../goldens/server_test/redirect_logs.golden | 2 +- packages/custom_lint/test/ignore_test.dart | 9 +- packages/custom_lint/test/run_plugin.dart | 2 + .../custom_lint/test/src/workspace_test.dart | 16 +- .../tools/analyzer_plugin/pubspec.yaml | 2 +- packages/custom_lint_builder/CHANGELOG.md | 5 + .../example/example_lint/pubspec.yaml | 2 +- .../custom_lint_builder/example/pubspec.yaml | 2 +- .../custom_lint_builder/lib/src/channel.dart | 2 + .../custom_lint_builder/lib/src/client.dart | 267 ++++++++++++-- .../custom_lint_builder/lib/src/ignore.dart | 12 +- packages/custom_lint_builder/pubspec.yaml | 2 +- .../pubspec_overrides.yaml | 4 +- packages/custom_lint_core/CHANGELOG.md | 8 + .../lib/src/change_reporter.dart | 9 +- .../custom_lint_core/lib/src/matcher.dart | 225 +++++++----- packages/custom_lint_core/pubspec.yaml | 3 +- .../custom_lint_core/test/assist_test.dart | 28 +- packages/custom_lint_core/test/fix_test.dart | 18 +- packages/custom_lint_core/test/snapshot.diff | 7 +- packages/custom_lint_core/test/snapshot2.diff | 7 +- .../test/type_checker_test.dart | 2 +- packages/lint_visitor_generator/pubspec.yaml | 2 +- pubspec.yaml | 2 +- 47 files changed, 1512 insertions(+), 308 deletions(-) create mode 100644 packages/custom_lint/test/goldens/fixes/add_ignore.diff create mode 100644 packages/custom_lint/test/goldens/fixes/fixes.diff create mode 100644 packages/custom_lint/test/goldens/fixes/multi_change.diff create mode 100644 packages/custom_lint/test/goldens/fixes/no_change.diff create mode 100644 packages/custom_lint/test/goldens/fixes/silenced_change.diff create mode 100644 packages/custom_lint/test/goldens/fixes/single_fix.diff create mode 100644 packages/custom_lint/test/goldens/fixes/update_ignore.diff diff --git a/README.md b/README.md index 67f3ec0d..6bd8e432 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ To create a custom lint, you will need two things: # pubspec.yaml name: my_custom_lint_package environment: - sdk: ">=2.16.0 <3.0.0" + sdk: ">=3.0.0 <4.0.0" dependencies: # we will use analyzer for inspecting Dart files @@ -166,7 +166,7 @@ For users to run custom_lint packages, there are a few steps: # The pubspec.yaml of an application using our lints name: example_app environment: - sdk: ">=2.16.0 <3.0.0" + sdk: ">=3.0.0 <4.0.0" dev_dependencies: custom_lint: diff --git a/packages/custom_lint/CHANGELOG.md b/packages/custom_lint/CHANGELOG.md index e18d97eb..bd3d0209 100644 --- a/packages/custom_lint/CHANGELOG.md +++ b/packages/custom_lint/CHANGELOG.md @@ -1,3 +1,7 @@ +## Unreleased patch + +- Added support for `--fix` + ## 0.5.11 - 2024-01-27 - Added support for `analysis_options.yaml` that are nt at the root of the project (thanks to @mrgnhnt96) diff --git a/packages/custom_lint/bin/custom_lint.dart b/packages/custom_lint/bin/custom_lint.dart index 6775331f..87ac97b6 100644 --- a/packages/custom_lint/bin/custom_lint.dart +++ b/packages/custom_lint/bin/custom_lint.dart @@ -39,6 +39,11 @@ Future entrypoint([List args = const []]) async { help: "Watches plugins' sources and perform a hot-reload on change", negatable: false, ) + ..addFlag( + 'fix', + help: 'Apply all possible fixes to the lint issues found.', + negatable: false, + ) ..addFlag( 'help', abbr: 'h', @@ -55,6 +60,7 @@ Future entrypoint([List args = const []]) async { } final watchMode = result['watch'] as bool; + final fix = result['fix'] as bool; final fatalInfos = result['fatal-infos'] as bool; final fatalWarnings = result['fatal-warnings'] as bool; final format = result['format'] as String; @@ -64,6 +70,7 @@ Future entrypoint([List args = const []]) async { watchMode: watchMode, fatalInfos: fatalInfos, fatalWarnings: fatalWarnings, + fix: fix, format: OutputFormatEnum.fromName(format), ); } diff --git a/packages/custom_lint/example/example_lint/lib/custom_lint_example_lint.dart b/packages/custom_lint/example/example_lint/lib/custom_lint_example_lint.dart index 515037c1..ff036548 100644 --- a/packages/custom_lint/example/example_lint/lib/custom_lint_example_lint.dart +++ b/packages/custom_lint/example/example_lint/lib/custom_lint_example_lint.dart @@ -112,7 +112,7 @@ class _MakeProviderFinalFix extends DartFix { message: 'Make provider final', // This represents how high-low should this quick-fix show-up in the list // of quick-fixes. - priority: 1, + priority: 10, ); // Our edit will consist of editing a Dart file, so we invoke "addDartFileEdit". diff --git a/packages/custom_lint/lib/custom_lint.dart b/packages/custom_lint/lib/custom_lint.dart index d6098381..f177d8ec 100644 --- a/packages/custom_lint/lib/custom_lint.dart +++ b/packages/custom_lint/lib/custom_lint.dart @@ -40,6 +40,7 @@ Future customLint({ bool fatalInfos = true, bool fatalWarnings = true, OutputFormatEnum format = OutputFormatEnum.plain, + bool fix = false, }) async { // Reset the code exitCode = 0; @@ -53,6 +54,7 @@ Future customLint({ fatalInfos: fatalInfos, fatalWarnings: fatalWarnings, format: format, + fix: fix, ); } catch (_) { exitCode = 1; @@ -68,10 +70,12 @@ Future _runServer( required bool fatalInfos, required bool fatalWarnings, required OutputFormatEnum format, + required bool fix, }) async { final customLintServer = await CustomLintServer.start( sendPort: channel.receivePort.sendPort, watchMode: watchMode, + fix: fix, workingDirectory: workingDirectory, // In the CLI, only show user defined lints. Errors & logs will be // rendered separately diff --git a/packages/custom_lint/lib/src/analyzer_plugin_starter.dart b/packages/custom_lint/lib/src/analyzer_plugin_starter.dart index d3cab76f..a0c2ffd0 100644 --- a/packages/custom_lint/lib/src/analyzer_plugin_starter.dart +++ b/packages/custom_lint/lib/src/analyzer_plugin_starter.dart @@ -13,9 +13,12 @@ void start(Iterable _, SendPort sendPort) { CustomLintServer.start( sendPort: sendPort, includeBuiltInLints: true, + // The IDE client should write to files, as what's visible in the editor + // may not be the same as what's on disk. + fix: false, // "start" may be run by `dart analyze`, in which case we don't want to // enable watch mode. There's no way to detect this, but this only matters - // in the CI. So we disble watch mode if we detect that we're in CI. + // in the CI. So we disable watch mode if we detect that we're in CI. // TODO enable hot-restart only if running plugin from source (excluding pub cache) watchMode: !isInCI, delegate: AnalyzerPluginCustomLintDelegate(), diff --git a/packages/custom_lint/lib/src/v2/custom_lint_analyzer_plugin.dart b/packages/custom_lint/lib/src/v2/custom_lint_analyzer_plugin.dart index a7b65ef6..35afb839 100644 --- a/packages/custom_lint/lib/src/v2/custom_lint_analyzer_plugin.dart +++ b/packages/custom_lint/lib/src/v2/custom_lint_analyzer_plugin.dart @@ -28,6 +28,7 @@ class CustomLintServer { required this.includeBuiltInLints, required this.delegate, required this.workingDirectory, + required this.fix, }); /// Start the server while also capturing prints and errors. @@ -38,6 +39,7 @@ class CustomLintServer { required SendPort sendPort, required bool watchMode, required bool includeBuiltInLints, + required bool fix, required CustomLintDelegate delegate, required Directory workingDirectory, }) { @@ -48,6 +50,7 @@ class CustomLintServer { () { server = CustomLintServer._( watchMode: watchMode, + fix: fix, includeBuiltInLints: includeBuiltInLints, delegate: delegate, workingDirectory: workingDirectory, @@ -96,6 +99,10 @@ class CustomLintServer { /// Whether plugins should be started in watch mode final bool watchMode; + /// If enabled, attempt to fix all issues found before reporting them. + /// Can only be enabled in the CLI. + final bool fix; + /// Whether plugins should include lints used for debugging. final bool includeBuiltInLints; diff --git a/packages/custom_lint/lib/src/v2/server_to_client_channel.dart b/packages/custom_lint/lib/src/v2/server_to_client_channel.dart index 83549726..733a6e02 100644 --- a/packages/custom_lint/lib/src/v2/server_to_client_channel.dart +++ b/packages/custom_lint/lib/src/v2/server_to_client_channel.dart @@ -209,6 +209,7 @@ void main(List args) async { runSocket( port: port, host: host, + fix: ${_server.fix}, includeBuiltInLints: ${_server.includeBuiltInLints}, {$plugins}, ); diff --git a/packages/custom_lint/lib/src/workspace.dart b/packages/custom_lint/lib/src/workspace.dart index c4de8607..c25dc1e3 100644 --- a/packages/custom_lint/lib/src/workspace.dart +++ b/packages/custom_lint/lib/src/workspace.dart @@ -562,7 +562,7 @@ publish_to: 'none' .toList(); final constraintCompatibleWithAllProjects = projectMeta.fold( - VersionConstraint.any, + VersionConstraint.parse('^3.0.0'), (acc, constraint) => acc.intersect(constraint.constraint), ); diff --git a/packages/custom_lint/pubspec.yaml b/packages/custom_lint/pubspec.yaml index 16de356c..42cab04e 100644 --- a/packages/custom_lint/pubspec.yaml +++ b/packages/custom_lint/pubspec.yaml @@ -32,6 +32,7 @@ dev_dependencies: build_runner: ^2.3.2 file: ^7.0.0 freezed: ^2.3.2 + glob: ^2.1.2 json_serializable: ^6.5.4 test: ^1.20.2 test_process: ^2.1.0 diff --git a/packages/custom_lint/test/cli_process_test.dart b/packages/custom_lint/test/cli_process_test.dart index 1dd52922..fffb2bd5 100644 --- a/packages/custom_lint/test/cli_process_test.dart +++ b/packages/custom_lint/test/cli_process_test.dart @@ -521,4 +521,131 @@ So, because custom_lint_client depends on test_lint from path, version solving f }); }); }); + + group('--fix', () { + final fixedPlugin = createPluginSource([ + TestLintRule( + code: 'oy', + message: 'Oy', + onVariable: 'if (node.name.toString().endsWith("fixed")) return;', + fixes: [TestLintFix(name: 'OyFix')], + ), + ]); + + test('Applies possible fixes and return remaining lints', () async { + final fixedPlugin = createPluginSource([ + TestLintRule( + code: 'oy', + message: 'Oy', + onVariable: 'if (node.name.toString().endsWith("fixed")) return;', + fixes: [TestLintFix(name: 'OyFix')], + ), + ]); + + final plugin = createPlugin(name: 'test_lint', main: fixedPlugin); + final plugin2 = createPlugin( + name: 'test_lint2', + main: helloWordPluginSource, + ); + + final app = createLintUsage( + name: 'test_app', + source: {'lib/main.dart': 'void fn() {}'}, + plugins: { + 'test_lint': plugin.uri, + 'test_lint2': plugin2.uri, + }, + ); + + final process = await Process.run( + 'dart', + [customLintBinPath, '--fix'], + workingDirectory: app.path, + ); + + expect(trimDependencyOverridesWarning(process.stderr), isEmpty); + + expect( + app.file('lib', 'main.dart').readAsStringSync(), + 'void fnfixed() {}', + ); + + expect(process.stdout, ''' +Analyzing... + + lib/main.dart:1:6 • Hello world • hello_world • INFO + +1 issue found. +'''); + expect(process.exitCode, 1); + }); + + test('Can fix all lints', () async { + final plugin = createPlugin(name: 'test_lint', main: fixedPlugin); + + final app = createLintUsage( + name: 'test_app', + source: {'lib/main.dart': 'void fn() {}'}, + plugins: {'test_lint': plugin.uri}, + ); + + final process = await Process.run( + 'dart', + [customLintBinPath, '--fix'], + workingDirectory: app.path, + ); + + expect(trimDependencyOverridesWarning(process.stderr), isEmpty); + + expect( + app.file('lib', 'main.dart').readAsStringSync(), + 'void fnfixed() {}', + ); + + expect(process.stdout, ''' +Analyzing... + +No issues found! +'''); + expect(process.exitCode, 0); + }); + + test( + 'Does not attempt at fixing the same lints again if a fix did not work', + () async { + final uselessFix = createPluginSource([ + TestLintRule( + code: 'oy', + message: 'Oy', + onVariable: 'if (node.name.toString().endsWith("fixed")) return;', + fixes: [TestLintFix(name: 'OyFix', nodeVisitor: '')], + ), + ]); + + final plugin = createPlugin(name: 'test_lint', main: uselessFix); + + final app = createLintUsage( + name: 'test_app', + source: {'lib/main.dart': 'void fn() {}'}, + plugins: {'test_lint': plugin.uri}, + ); + + final process = await Process.run( + 'dart', + [customLintBinPath, '--fix'], + workingDirectory: app.path, + ); + + expect(trimDependencyOverridesWarning(process.stderr), isEmpty); + + expect(process.stdout, ''' +Analyzing... + + lib/main.dart:1:6 • Oy • oy • INFO + +1 issue found. +'''); + expect(process.exitCode, 1); + }); + }); } diff --git a/packages/custom_lint/test/cli_test.dart b/packages/custom_lint/test/cli_test.dart index 55a232d2..dc205dcd 100644 --- a/packages/custom_lint/test/cli_test.dart +++ b/packages/custom_lint/test/cli_test.dart @@ -255,7 +255,7 @@ invalid; ^^^^^^^ '''), matchIgnoringAnsi(contains, ''' -lib/custom_lint_client.dart:15:29: Error: Undefined name 'createPlugin'. +lib/custom_lint_client.dart:16:29: Error: Undefined name 'createPlugin'. {'test_lint': test_lint.createPlugin, ^^^^^^^^^^^^ '''), @@ -377,7 +377,7 @@ int x = 'oy'; ^ '''), matchIgnoringAnsi(contains, ''' -lib/custom_lint_client.dart:17:26: Error: Undefined name 'createPlugin'. +lib/custom_lint_client.dart:18:26: Error: Undefined name 'createPlugin'. 'test_lint2': test_lint2.createPlugin, ^^^^^^^^^^^^ '''), diff --git a/packages/custom_lint/test/create_project.dart b/packages/custom_lint/test/create_project.dart index dff3c059..3675115c 100644 --- a/packages/custom_lint/test/create_project.dart +++ b/packages/custom_lint/test/create_project.dart @@ -42,87 +42,40 @@ class TestLintRule { final String ruleMembers; final List fixes; final ErrorSeverity errorSeverity; -} - -class TestLintFix { - TestLintFix({required this.name}); - - final String name; -} - -String createPluginSource(List rules) { - final buffer = StringBuffer(''' -import 'package:analyzer/dart/element/element.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:analyzer/dart/analysis/results.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:analyzer/error/error.dart'; - -PluginBase createPlugin() => _Plugin(); - -class _Plugin extends PluginBase { - @override - List getLintRules(CustomLintConfigs configs) => [ -'''); - - buffer.writeAll(rules.map((e) => '${e.code}()'), ','); - - buffer.write(']; }'); - for (final rule in rules) { - final fixes = rule.fixes.isEmpty + void run(StringBuffer buffer) { + final fixesCode = fixes.isEmpty ? '' : ''' @override -List getFixes() => [${rule.fixes.map((e) => '${e.name}()').join(',')}]; +List getFixes() => [${fixes.map((e) => '${e.name}()').join(',')}]; '''; - for (final fix in rule.fixes) { - buffer.write(''' -class ${fix.name} extends DartFix { - @override - void run( - CustomLintResolver resolver, - ChangeReporter reporter, - CustomLintContext context, - AnalysisError analysisError, - List others, - ) { - context.registry.addVariableDeclarationList((node) { - if (!analysisError.sourceRange.intersects(node.sourceRange)) return; - - final changeBuilder = reporter.createChangeBuilder( - priority: 1, - message: 'Fix ${rule.code}', - ); - changeBuilder.addDartFileEdit((builder) {}); - }); - } -} -'''); + for (final fix in fixes) { + fix.write(buffer, this); } buffer.write(''' -class ${rule.code} extends DartLintRule { - ${rule.code}() +class $code extends DartLintRule { + $code() : super( - code: LintCode(name: '${rule.code}', - problemMessage: '${rule.message}', - errorSeverity: ErrorSeverity.${rule.errorSeverity.displayName.toUpperCase()}), + code: LintCode(name: '$code', + problemMessage: '$message', + errorSeverity: ErrorSeverity.${errorSeverity.displayName.toUpperCase()}), ); -$fixes -${rule.ruleMembers} +$fixesCode +$ruleMembers '''); - if (rule.startUp.isNotEmpty) { + if (startUp.isNotEmpty) { buffer.write(''' @override Future startUp( CustomLintResolver resolver, CustomLintContext context, ) async { - ${rule.startUp} + $startUp } '''); } @@ -131,9 +84,9 @@ ${rule.ruleMembers} ''' @override void run(CustomLintResolver resolver, ErrorReporter reporter, CustomLintContext context) { - ${rule.onRun} + $onRun context.registry.addFunctionDeclaration((node) { - ${rule.onVariable} + $onVariable reporter.reportErrorForToken(code, node.name); }); } @@ -141,6 +94,71 @@ ${rule.ruleMembers} ''', ); } +} + +class TestLintFix { + TestLintFix({ + required this.name, + this.nodeVisitor, + }); + + final String name; + final String? nodeVisitor; + + void write(StringBuffer buffer, TestLintRule rule) { + buffer.write(''' +class $name extends DartFix { + @override + void run( + CustomLintResolver resolver, + ChangeReporter reporter, + CustomLintContext context, + AnalysisError analysisError, + List others, + ) { + context.registry.addFunctionDeclaration((node) { + if (!analysisError.sourceRange.intersects(node.sourceRange)) return; + + final changeBuilder = reporter.createChangeBuilder( + priority: 1, + message: 'Fix ${rule.code}', + ); + + ${nodeVisitor ?? r''' + changeBuilder.addDartFileEdit((builder) { + builder.addSimpleReplacement(node.name.sourceRange, '${node.name}fixed'); + }); +'''} + }); + } +} +'''); + } +} + +String createPluginSource(List rules) { + final buffer = StringBuffer(''' +import 'package:analyzer/dart/element/element.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:analyzer/dart/analysis/results.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:path/path.dart' as p; + +PluginBase createPlugin() => _Plugin(); + +class _Plugin extends PluginBase { + @override + List getLintRules(CustomLintConfigs configs) => [ +'''); + + buffer.writeAll(rules.map((e) => '${e.code}()'), ','); + + buffer.write(']; }'); + + for (final rule in rules) { + rule.run(buffer); + } return buffer.toString(); } @@ -173,7 +191,7 @@ version: 0.0.1 publish_to: none environment: - sdk: ">=2.17.0 <4.0.0" + sdk: ">=3.0.0 <4.0.0" dependencies: analyzer: any @@ -221,7 +239,7 @@ version: 0.0.1 publish_to: none environment: - sdk: ">=2.17.0 <4.0.0" + sdk: ">=3.0.0 <4.0.0" dependencies: analyzer: any @@ -263,19 +281,19 @@ String createPackageConfig({ 'name': plugin.key, 'rootUri': plugin.value.toString(), 'packageUri': 'lib/', - 'languageVersion': '2.17', + 'languageVersion': '3.0', }, { 'name': name, 'rootUri': '../', 'packageUri': 'lib/', - 'languageVersion': '2.17', + 'languageVersion': '3.0', }, { 'name': 'custom_lint', 'rootUri': 'file://${PeerProjectMeta.current.customLintPath}', 'packageUri': 'lib/', - 'languageVersion': '2.17', + 'languageVersion': '3.0', }, // Custom lint builder is always a transitive dev dependency if it is used, // so it will be in the package config @@ -283,7 +301,7 @@ String createPackageConfig({ 'name': 'custom_lint_builder', 'rootUri': 'file://${PeerProjectMeta.current.customLintBuilderPath}', 'packageUri': 'lib/', - 'languageVersion': '2.17', + 'languageVersion': '3.0', }, // Custom lint core is always a transitive dev dependency if it is used, // so it will be in the package config @@ -291,7 +309,7 @@ String createPackageConfig({ 'name': 'custom_lint_core', 'rootUri': 'file://${PeerProjectMeta.current.customLintCorePath}', 'packageUri': 'lib/', - 'languageVersion': '2.17', + 'languageVersion': '3.0', }, ], }); diff --git a/packages/custom_lint/test/fixes_test.dart b/packages/custom_lint/test/fixes_test.dart index 1ff3fded..e0935c5c 100644 --- a/packages/custom_lint/test/fixes_test.dart +++ b/packages/custom_lint/test/fixes_test.dart @@ -1,11 +1,18 @@ -import 'package:analyzer_plugin/protocol/protocol_generated.dart'; -import 'package:path/path.dart'; +import 'dart:io'; + +import 'package:custom_lint/src/package_utils.dart'; +import 'package:path/path.dart' as p; import 'package:test/test.dart'; import 'create_project.dart'; +import 'goldens.dart'; import 'run_plugin.dart'; -final pluginWithFixSource = createPluginSource([ +final fixlessPlugin = createPluginSource([ + TestLintRule(code: 'hello_world', message: 'Hello world'), +]); + +final fixedPlugin = createPluginSource([ TestLintRule( code: 'hello_world', message: 'Hello world', @@ -13,61 +20,329 @@ final pluginWithFixSource = createPluginSource([ ), ]); +final noChangeFixPlugin = createPluginSource([ + TestLintRule( + code: 'hello_world', + message: 'Hello world', + fixes: [ + TestLintFix( + name: 'HelloWorldFix', + nodeVisitor: '', + ), + ], + ), +]); + +final multiChangeFixPlugin = createPluginSource([ + TestLintRule( + code: 'hello_world', + message: 'Hello world', + fixes: [ + TestLintFix( + name: 'HelloWorldFix', + nodeVisitor: r''' + changeBuilder.addDartFileEdit( + (builder) { + builder.addSimpleReplacement(node.name.sourceRange, '${node.name}fixed'); + }, + ); + changeBuilder.addDartFileEdit( + customPath: '${p.dirname(resolver.path)}/src/hello_world.dart', + (builder) { + builder.addSimpleReplacement(node.name.sourceRange, '${node.name}fixed'); + }, + ); +''', + ), + ], + ), +]); + void main() { - test('Handles fixes`', () async { - final plugin = createPlugin(name: 'test_lint', main: pluginWithFixSource); + test('Can emit fixes', () async { + final plugin = createPlugin( + name: 'test_lint', + main: fixedPlugin, + ); + const mainSource = ''' +void fn() {} + +void fn2() {} +'''; final app = createLintUsage( source: { - 'lib/main.dart': ''' + 'lib/main.dart': mainSource, + }, + plugins: {'test_lint': plugin.uri}, + name: 'test_app', + ); + final mainPath = p.join(app.path, 'lib', 'main.dart'); + + final runner = await startRunnerForApp(app); + await runner.channel.lints.first; + + final fixes = runner.getFixes(mainPath, 6); + final fixes2 = runner.getFixes(mainPath, 20); + + expectMatchesGoldenFixes( + [await fixes, await fixes2] + .expand((e) => e.fixes) + .expand((e) => e.fixes) + .where( + (e) => + e.change.id != 'ignore_for_file' && + e.change.id != 'ignore_for_line', + ), + sources: ({'**/*': mainSource}, relativePath: app.path), + file: Directory.current.file( + 'test', + 'goldens', + 'fixes', + 'fixes.diff', + ), + ); + }); + + test('Fix-all is not present if a lint only has a single issue', () async { + final plugin = createPlugin(name: 'test_lint', main: fixedPlugin); + + const mainSource = ''' void fn() {} -''', +'''; + final app = createLintUsage( + source: { + 'lib/main.dart': mainSource, }, plugins: {'test_lint': plugin.uri}, name: 'test_app', ); - final mainPath = join(app.path, 'lib', 'main.dart'); + final mainPath = p.join(app.path, 'lib', 'main.dart'); final runner = await startRunnerForApp(app); + await runner.channel.lints.first; + + final fixes = await runner.getFixes(mainPath, 6); - expect( - await runner.channel.lints.first, - isA() - .having((e) => e.file, 'file', mainPath) - .having((e) => e.errors.single.location.offset, 'offset', 5) - .having((e) => e.errors.single.location.length, 'length', 2), + expectMatchesGoldenFixes( + fixes.fixes.expand((e) => e.fixes).where( + (e) => + e.change.id != 'ignore_for_file' && + e.change.id != 'ignore_for_line', + ), + sources: ({'**/*': mainSource}, relativePath: app.path), + file: Directory.current.file( + 'test', + 'goldens', + 'fixes', + 'single_fix.diff', + ), ); + }); - final fixesBeforeOffset = runner.getFixes(mainPath, 4); - final fixesAtStartOffset = runner.getFixes(mainPath, 5); - final fixesAtMiddleOffset = runner.getFixes(mainPath, 6); - final fixesAtEndOffset = runner.getFixes(mainPath, 7); - final fixesAfterOffset = runner.getFixes(mainPath, 8); + test('Fix-all does not apply to silenced lints', () async { + final plugin = createPlugin(name: 'test_lint', main: fixedPlugin); + + const mainSource = ''' +void fn() {} - expect( - await Future.wait([fixesAfterOffset, fixesBeforeOffset]), - everyElement( - isA().having((e) => e.fixes, 'fixes', isEmpty), +void fn2() {} + +// ignore: hello_world +void fn3() {} + +// expect_lint: hello_world +void fn4() {} +'''; + final app = createLintUsage( + source: { + 'lib/main.dart': mainSource, + }, + plugins: {'test_lint': plugin.uri}, + name: 'test_app', + ); + final mainPath = p.join(app.path, 'lib', 'main.dart'); + + final runner = await startRunnerForApp(app); + await runner.channel.lints.first; + + final fixes = await runner.getFixes(mainPath, 6); + + expectMatchesGoldenFixes( + fixes.fixes.expand((e) => e.fixes).where( + (e) => + e.change.id != 'ignore_for_file' && + e.change.id != 'ignore_for_line', + ), + sources: ({'**/*': mainSource}, relativePath: app.path), + file: Directory.current.file( + 'test', + 'goldens', + 'fixes', + 'silenced_change.diff', ), ); + }); + + test('Supports fixes with no changes', () async { + final plugin = createPlugin(name: 'test_lint', main: noChangeFixPlugin); - expect( - await Future.wait([ - fixesAtStartOffset, - fixesAtMiddleOffset, - fixesAtEndOffset, - ]), - everyElement( - isA().having((e) => e.fixes, 'fixes', hasLength(1)), + const mainSource = ''' +void fn() {} + +void fn2() {} +'''; + final app = createLintUsage( + source: { + 'lib/main.dart': mainSource, + }, + plugins: {'test_lint': plugin.uri}, + name: 'test_app', + ); + final mainPath = p.join(app.path, 'lib', 'main.dart'); + + final runner = await startRunnerForApp(app); + await runner.channel.lints.first; + + final fixes = runner.getFixes(mainPath, 6); + final fixes2 = runner.getFixes(mainPath, 20); + + expectMatchesGoldenFixes( + [await fixes, await fixes2] + .expand((e) => e.fixes) + .expand((e) => e.fixes) + .where( + (e) => + e.change.id != 'ignore_for_file' && + e.change.id != 'ignore_for_line', + ), + sources: ({'**/*': mainSource}, relativePath: app.path), + file: Directory.current.file( + 'test', + 'goldens', + 'fixes', + 'no_change.diff', + ), + ); + }); + + test('Supports fixes that emits multiple changes', () async { + final plugin = createPlugin(name: 'test_lint', main: multiChangeFixPlugin); + + const mainSource = ''' +void fn() {} + +void fn2() {} +'''; + final app = createLintUsage( + source: { + 'lib/main.dart': mainSource, + }, + plugins: {'test_lint': plugin.uri}, + name: 'test_app', + ); + final mainPath = p.join(app.path, 'lib', 'main.dart'); + + final runner = await startRunnerForApp(app); + await runner.channel.lints.first; + + final fixes = runner.getFixes(mainPath, 6); + final fixes2 = runner.getFixes(mainPath, 20); + + expectMatchesGoldenFixes( + [await fixes, await fixes2] + .expand((e) => e.fixes) + .expand((e) => e.fixes) + .where( + (e) => + e.change.id != 'ignore_for_file' && + e.change.id != 'ignore_for_line', + ), + sources: ({'**/*': mainSource}, relativePath: app.path), + file: Directory.current.file( + 'test', + 'goldens', + 'fixes', + 'multi_change.diff', + ), + ); + }); + + test('Can add new ignores', () async { + final plugin = createPlugin(name: 'test_lint', main: fixlessPlugin); + + const mainSource = ''' +void fn() {} + +void fn2() {} + + void fn3() {} +'''; + final app = createLintUsage( + source: { + 'lib/main.dart': mainSource, + }, + plugins: {'test_lint': plugin.uri}, + name: 'test_app', + ); + final mainPath = p.join(app.path, 'lib', 'main.dart'); + + final runner = await startRunnerForApp(app); + await runner.channel.lints.first; + + final fixes = runner.getFixes(mainPath, 6); + final fixes2 = runner.getFixes(mainPath, 20); + final fixes3 = runner.getFixes(mainPath, 37); + + expectMatchesGoldenFixes( + [await fixes, await fixes2, await fixes3] + .expand((e) => e.fixes) + .expand((e) => e.fixes), + sources: ({'**/*': mainSource}, relativePath: app.path), + file: Directory.current.file( + 'test', + 'goldens', + 'fixes', + 'add_ignore.diff', ), ); + }); + + test('Can update existing ignores', () async { + final plugin = createPlugin(name: 'test_lint', main: fixlessPlugin); + + const mainSource = ''' +// ignore: hello_world +void fn() {} + +// ignore_for_file: foo +// ignore: foo +void fn2() {} +'''; + final app = createLintUsage( + source: { + 'lib/main.dart': mainSource, + }, + plugins: {'test_lint': plugin.uri}, + name: 'test_app', + ); + final mainPath = p.join(app.path, 'lib', 'main.dart'); - expect(runner.channel.lints, emitsDone); + final runner = await startRunnerForApp(app); + await runner.channel.lints.first; - // Closing so that previous error matchers relying on stream - // closing can complete - await runner.close(); + final fixes = runner.getFixes(mainPath, 30); + final fixes2 = runner.getFixes(mainPath, 84); - expect(plugin.log.existsSync(), false); + expectMatchesGoldenFixes( + [await fixes, await fixes2].expand((e) => e.fixes).expand((e) => e.fixes), + sources: ({'**/*': mainSource}, relativePath: app.path), + file: Directory.current.file( + 'test', + 'goldens', + 'fixes', + 'update_ignore.diff', + ), + ); }); } diff --git a/packages/custom_lint/test/goldens.dart b/packages/custom_lint/test/goldens.dart index 52d6ade1..ba9e0038 100644 --- a/packages/custom_lint/test/goldens.dart +++ b/packages/custom_lint/test/goldens.dart @@ -1,51 +1,190 @@ import 'dart:convert'; import 'dart:io'; +import 'dart:math'; +import 'package:analyzer/source/line_info.dart'; +import 'package:analyzer_plugin/protocol/protocol_common.dart'; import 'package:analyzer_plugin/protocol/protocol_generated.dart'; -import 'package:custom_lint/src/package_utils.dart'; +import 'package:collection/collection.dart'; +import 'package:glob/glob.dart'; +import 'package:path/path.dart' as p; import 'package:test/test.dart'; -final _fixesGoldenFile = - Directory.current.file('test', 'goldens', 'ignore_quick_fix.json'); +@Deprecated('do not commit') +void saveGoldensFixes( + Iterable fixes, { + (Map, {String relativePath})? sources, + required File file, +}) { + file.createSync(recursive: true); -Object? _encodePrioritizedSourceChange(PrioritizedSourceChange change) { - final json = change.toJson(); + file.writeAsStringSync( + _encodePrioritizedSourceChanges( + fixes, + sources: sources?.$1, + relativePath: sources?.relativePath, + ), + ); +} - final jsonChange = json['change']! as Map; - final jsonEdits = jsonChange['edits']! as List; +void expectMatchesGoldenFixes( + Iterable fixes, { + (Map, {String relativePath})? sources, + required File file, +}) { + expect( + _encodePrioritizedSourceChanges( + fixes, + sources: sources?.$1, + relativePath: sources?.relativePath, + ), + file.readAsStringSync(), + ); +} - for (final jsonEdit in jsonEdits) { - jsonEdit as Map; - jsonEdit.remove('file'); - } +// forked from custom-lint-core +String _encodePrioritizedSourceChanges( + Iterable changes, { + JsonEncoder? encoder, + Map? sources, + String? relativePath, +}) { + if (sources != null) { + final buffer = StringBuffer(); - return json; -} + for (final prioritizedSourceChange in changes) { + buffer.writeln('Message: `${prioritizedSourceChange.change.message}`'); + buffer.writeln('Priority: ${prioritizedSourceChange.priority}'); + if (prioritizedSourceChange.change.id != null) { + buffer.writeln('Id: `${prioritizedSourceChange.change.id}`'); + } + if (prioritizedSourceChange.change.selection case final selection?) { + buffer.writeln( + 'Selection: offset ${selection.offset} ; ' + 'file: `${selection.file}`; ' + 'length: ${prioritizedSourceChange.change.selectionLength}', + ); + } -Object? _encodeAnalysisErrorFixes(AnalysisErrorFixes fixes) { - final json = fixes.toJson(); + final files = prioritizedSourceChange.change.edits + .map((e) => p.normalize(p.relative(e.file, from: relativePath))) + .toSet() + .sortedBy((a) => a); - final editedPrioritizedSourceChanges = - fixes.fixes.map(_encodePrioritizedSourceChange).toList(); + for (final file in files) { + final source = sources.entries + .firstWhereOrNull( + (e) => + Glob(e.key).matches(file) || + // workaround to https://github.com/dart-lang/glob/issues/72 + Glob('/${e.key}').matches(file), + ) + ?.value; + if (source == null) { + throw StateError('No source found for file: $file'); + } - final jsonError = json['error']! as Map; - final jsonLocation = jsonError['location']! as Map; - jsonLocation.remove('file'); + final sourceLineInfo = LineInfo.fromContent(source); - json['fixes'] = editedPrioritizedSourceChanges; + final output = SourceEdit.applySequence( + source, + prioritizedSourceChange.change.edits + .expand((element) => element.edits), + ); - return json; -} + final outputLineInfo = LineInfo.fromContent(output); -void saveGoldensFixes(List fixes) { - _fixesGoldenFile.writeAsStringSync( - jsonEncode(fixes.map(_encodeAnalysisErrorFixes).toList()), - ); -} + // Get the offset of the first changed character between output and source. + var firstDiffOffset = 0; + for (; firstDiffOffset < source.length; firstDiffOffset++) { + if (source[firstDiffOffset] != output[firstDiffOffset]) { + break; + } + } -void expectMatchesGoldenFixes(List fixes) { - expect( - jsonEncode(fixes.map(_encodeAnalysisErrorFixes).toList()), - _fixesGoldenFile.readAsStringSync(), - ); + // Get the last changed character offset between output and source. + var endSourceOffset = source.length - 1; + var endOutputOffset = output.length - 1; + for (; + endOutputOffset > firstDiffOffset && + endSourceOffset > firstDiffOffset; + endOutputOffset--, endSourceOffset--) { + if (source[endSourceOffset] != output[endOutputOffset]) { + break; + } + } + + final firstChangedLine = + sourceLineInfo.getLocation(firstDiffOffset).lineNumber - 1; + + void writeDiff({ + required String file, + required LineInfo lineInfo, + required int endOffset, + required String token, + required int leadingCount, + required int trailingCount, + }) { + final lastChangedLine = + lineInfo.getLocation(endOffset).lineNumber - 1; + final endLine = + min(lastChangedLine + trailingCount, lineInfo.lineCount - 1); + for (var line = max(0, firstChangedLine - leadingCount); + line <= endLine; + line++) { + final changed = line >= firstChangedLine && line <= lastChangedLine; + if (changed) buffer.write(token); + + final endOfSource = !(line + 1 < lineInfo.lineCount); + + buffer.write( + file.substring( + lineInfo.getOffsetOfLine(line), + endOfSource ? null : lineInfo.getOffsetOfLine(line + 1) - 1, + ), + ); + if (!endOfSource) buffer.writeln(); + } + } + + buffer.writeln('Diff for file `$file:${firstChangedLine + 1}`:'); + buffer.writeln('```'); + writeDiff( + file: source, + lineInfo: sourceLineInfo, + endOffset: endSourceOffset, + leadingCount: 2, + trailingCount: 0, + token: '- ', + ); + + writeDiff( + file: output, + lineInfo: outputLineInfo, + endOffset: endOutputOffset, + leadingCount: 0, + trailingCount: 2, + token: '+ ', + ); + buffer.writeln('```'); + } + + buffer.writeln('---'); + } + + return buffer.toString(); + } + + final json = changes.map((e) => e.toJson()).toList(); + // Remove all "file" references from the json. + for (final change in json) { + final changeMap = change['change']! as Map; + final edits = changeMap['edits']! as List; + for (final edit in edits.cast>()) { + edit.remove('file'); + } + } + + encoder ??= const JsonEncoder.withIndent(' '); + return encoder.convert(json); } diff --git a/packages/custom_lint/test/goldens/fixes/add_ignore.diff b/packages/custom_lint/test/goldens/fixes/add_ignore.diff new file mode 100644 index 00000000..4603ca61 --- /dev/null +++ b/packages/custom_lint/test/goldens/fixes/add_ignore.diff @@ -0,0 +1,74 @@ +Message: `Ignore "hello_world" for line` +Priority: 1 +Id: `ignore_for_line` +Diff for file `lib/main.dart:1`: +``` +- void fn() {} ++ // ignore: hello_world ++ void fn() {} + +void fn2() {} +``` +--- +Message: `Ignore "hello_world" for file` +Priority: 0 +Id: `ignore_for_file` +Diff for file `lib/main.dart:1`: +``` +- void fn() {} ++ // ignore_for_file: hello_world ++ void fn() {} + +void fn2() {} +``` +--- +Message: `Ignore "hello_world" for line` +Priority: 1 +Id: `ignore_for_line` +Diff for file `lib/main.dart:3`: +``` +void fn() {} + +- void fn2() {} ++ // ignore: hello_world ++ void fn2() {} + + void fn3() {} +``` +--- +Message: `Ignore "hello_world" for file` +Priority: 0 +Id: `ignore_for_file` +Diff for file `lib/main.dart:1`: +``` +- void fn() {} ++ // ignore_for_file: hello_world ++ void fn() {} + +void fn2() {} +``` +--- +Message: `Ignore "hello_world" for line` +Priority: 1 +Id: `ignore_for_line` +Diff for file `lib/main.dart:5`: +``` +void fn2() {} + +- void fn3() {} ++ // ignore: hello_world ++ void fn3() {} +``` +--- +Message: `Ignore "hello_world" for file` +Priority: 0 +Id: `ignore_for_file` +Diff for file `lib/main.dart:1`: +``` +- void fn() {} ++ // ignore_for_file: hello_world ++ void fn() {} + +void fn2() {} +``` +--- diff --git a/packages/custom_lint/test/goldens/fixes/fixes.diff b/packages/custom_lint/test/goldens/fixes/fixes.diff new file mode 100644 index 00000000..46ad1a9f --- /dev/null +++ b/packages/custom_lint/test/goldens/fixes/fixes.diff @@ -0,0 +1,44 @@ +Message: `Fix hello_world` +Priority: 1 +Diff for file `lib/main.dart:1`: +``` +- void fn() {} ++ void fnfixed() {} + +void fn2() {} +``` +--- +Message: `Fix all "hello_world"` +Priority: 0 +Diff for file `lib/main.dart:1`: +``` +- void fn() {} +- +- void fn2() {} ++ void fnfixed() {} ++ ++ void fn2fixed() {} +``` +--- +Message: `Fix hello_world` +Priority: 1 +Diff for file `lib/main.dart:3`: +``` +void fn() {} + +- void fn2() {} ++ void fn2fixed() {} +``` +--- +Message: `Fix all "hello_world"` +Priority: 0 +Diff for file `lib/main.dart:1`: +``` +- void fn() {} +- +- void fn2() {} ++ void fnfixed() {} ++ ++ void fn2fixed() {} +``` +--- diff --git a/packages/custom_lint/test/goldens/fixes/multi_change.diff b/packages/custom_lint/test/goldens/fixes/multi_change.diff new file mode 100644 index 00000000..3fc490b1 --- /dev/null +++ b/packages/custom_lint/test/goldens/fixes/multi_change.diff @@ -0,0 +1,34 @@ +Message: `Fix hello_world` +Priority: 1 +Diff for file `lib/main.dart:1`: +``` +- void fn() {} ++ void fnfixedfixed() {} + +void fn2() {} +``` +Diff for file `lib/src/hello_world.dart:1`: +``` +- void fn() {} ++ void fnfixedfixed() {} + +void fn2() {} +``` +--- +Message: `Fix hello_world` +Priority: 1 +Diff for file `lib/main.dart:3`: +``` +void fn() {} + +- void fn2() {} ++ void fn2fixedfixed() {} +``` +Diff for file `lib/src/hello_world.dart:3`: +``` +void fn() {} + +- void fn2() {} ++ void fn2fixedfixed() {} +``` +--- diff --git a/packages/custom_lint/test/goldens/fixes/no_change.diff b/packages/custom_lint/test/goldens/fixes/no_change.diff new file mode 100644 index 00000000..d7b05d42 --- /dev/null +++ b/packages/custom_lint/test/goldens/fixes/no_change.diff @@ -0,0 +1,12 @@ +Message: `Fix hello_world` +Priority: 1 +--- +Message: `Fix all "hello_world"` +Priority: 0 +--- +Message: `Fix hello_world` +Priority: 1 +--- +Message: `Fix all "hello_world"` +Priority: 0 +--- diff --git a/packages/custom_lint/test/goldens/fixes/silenced_change.diff b/packages/custom_lint/test/goldens/fixes/silenced_change.diff new file mode 100644 index 00000000..132b58b0 --- /dev/null +++ b/packages/custom_lint/test/goldens/fixes/silenced_change.diff @@ -0,0 +1,24 @@ +Message: `Fix hello_world` +Priority: 1 +Diff for file `lib/main.dart:1`: +``` +- void fn() {} ++ void fnfixed() {} + +void fn2() {} +``` +--- +Message: `Fix all "hello_world"` +Priority: 0 +Diff for file `lib/main.dart:1`: +``` +- void fn() {} +- +- void fn2() {} ++ void fnfixed() {} ++ ++ void fn2fixed() {} + +// ignore: hello_world +``` +--- diff --git a/packages/custom_lint/test/goldens/fixes/single_fix.diff b/packages/custom_lint/test/goldens/fixes/single_fix.diff new file mode 100644 index 00000000..77960eb8 --- /dev/null +++ b/packages/custom_lint/test/goldens/fixes/single_fix.diff @@ -0,0 +1,8 @@ +Message: `Fix hello_world` +Priority: 1 +Diff for file `lib/main.dart:1`: +``` +- void fn() {} ++ void fnfixed() {} +``` +--- diff --git a/packages/custom_lint/test/goldens/fixes/update_ignore.diff b/packages/custom_lint/test/goldens/fixes/update_ignore.diff new file mode 100644 index 00000000..c037b634 --- /dev/null +++ b/packages/custom_lint/test/goldens/fixes/update_ignore.diff @@ -0,0 +1,25 @@ +Message: `Ignore "hello_world" for line` +Priority: 1 +Id: `ignore_for_line` +Diff for file `lib/main.dart:5`: +``` + +// ignore_for_file: foo +- // ignore: foo ++ // ignore: foo, hello_world +void fn2() {} +``` +--- +Message: `Ignore "hello_world" for file` +Priority: 0 +Id: `ignore_for_file` +Diff for file `lib/main.dart:4`: +``` +void fn() {} + +- // ignore_for_file: foo ++ // ignore_for_file: foo, hello_world +// ignore: foo +void fn2() {} +``` +--- diff --git a/packages/custom_lint/test/goldens/ignore_quick_fix.json b/packages/custom_lint/test/goldens/ignore_quick_fix.json index 2713c4af..9d7ed37e 100644 --- a/packages/custom_lint/test/goldens/ignore_quick_fix.json +++ b/packages/custom_lint/test/goldens/ignore_quick_fix.json @@ -1 +1,82 @@ -[{"error":{"severity":"INFO","type":"LINT","location":{"offset":5,"length":2,"startLine":1,"startColumn":6,"endLine":1,"endColumn":8},"message":"Hello world","code":"hello_world"},"fixes":[{"priority":35,"change":{"message":"Ignore \"hello_world\" for line","edits":[{"fileStamp":0,"edits":[{"offset":0,"length":0,"replacement":"// ignore: hello_world\n"}]}],"linkedEditGroups":[]}},{"priority":34,"change":{"message":"Ignore \"hello_world\" for file","edits":[{"fileStamp":0,"edits":[{"offset":0,"length":0,"replacement":"// ignore_for_file: hello_world\n"}]}],"linkedEditGroups":[]}}]},{"error":{"severity":"INFO","type":"LINT","location":{"offset":5,"length":2,"startLine":1,"startColumn":6,"endLine":1,"endColumn":8},"message":"Foo","code":"foo"},"fixes":[{"priority":35,"change":{"message":"Ignore \"foo\" for line","edits":[{"fileStamp":0,"edits":[{"offset":0,"length":0,"replacement":"// ignore: foo\n"}]}],"linkedEditGroups":[]}},{"priority":34,"change":{"message":"Ignore \"foo\" for file","edits":[{"fileStamp":0,"edits":[{"offset":0,"length":0,"replacement":"// ignore_for_file: foo\n"}]}],"linkedEditGroups":[]}}]}] \ No newline at end of file +[ + { + "priority": 1, + "change": { + "message": "Ignore \"hello_world\" for line", + "edits": [ + { + "fileStamp": 0, + "edits": [ + { + "offset": 0, + "length": 0, + "replacement": "// ignore: hello_world\n" + } + ] + } + ], + "linkedEditGroups": [], + "id": "ignore_for_line" + } + }, + { + "priority": 0, + "change": { + "message": "Ignore \"hello_world\" for file", + "edits": [ + { + "fileStamp": 0, + "edits": [ + { + "offset": 0, + "length": 0, + "replacement": "// ignore_for_file: hello_world\n" + } + ] + } + ], + "linkedEditGroups": [], + "id": "ignore_for_file" + } + }, + { + "priority": 1, + "change": { + "message": "Ignore \"foo\" for line", + "edits": [ + { + "fileStamp": 0, + "edits": [ + { + "offset": 0, + "length": 0, + "replacement": "// ignore: foo\n" + } + ] + } + ], + "linkedEditGroups": [], + "id": "ignore_for_line" + } + }, + { + "priority": 0, + "change": { + "message": "Ignore \"foo\" for file", + "edits": [ + { + "fileStamp": 0, + "edits": [ + { + "offset": 0, + "length": 0, + "replacement": "// ignore_for_file: foo\n" + } + ] + } + ], + "linkedEditGroups": [], + "id": "ignore_for_file" + } + } +] \ No newline at end of file diff --git a/packages/custom_lint/test/goldens/server_test/redirect_logs.golden b/packages/custom_lint/test/goldens/server_test/redirect_logs.golden index 29dfd2d6..dbb3ec8b 100644 --- a/packages/custom_lint/test/goldens/server_test/redirect_logs.golden +++ b/packages/custom_lint/test/goldens/server_test/redirect_logs.golden @@ -1,4 +1,4 @@ [hello_world] 1990-01-01T00:00:00.000 Hello world [hello_world] 1990-01-01T00:00:00.000 Plugin hello_world threw while analyzing app/lib/another.dart: [hello_world] 1990-01-01T00:00:00.000 Bad state: fail -[hello_world] 1990-01-01T00:00:00.000 #0 hello_world.run. (package:test_lint/test_lint.dart:28:3) \ No newline at end of file +[hello_world] 1990-01-01T00:00:00.000 #0 hello_world.run. (package:test_lint/test_lint.dart:29:3) \ No newline at end of file diff --git a/packages/custom_lint/test/ignore_test.dart b/packages/custom_lint/test/ignore_test.dart index 1d03f6a3..76da424c 100644 --- a/packages/custom_lint/test/ignore_test.dart +++ b/packages/custom_lint/test/ignore_test.dart @@ -1,3 +1,5 @@ +import 'dart:io'; + import 'package:analyzer_plugin/protocol/protocol_common.dart'; import 'package:analyzer_plugin/protocol/protocol_generated.dart'; import 'package:custom_lint/src/package_utils.dart'; @@ -61,9 +63,10 @@ void fn2() {} unorderedEquals(['Ignore "foo" for line', 'Ignore "foo" for file']), ); - // saveGoldensFixes(fixes); - - expectMatchesGoldenFixes(fixes); + expectMatchesGoldenFixes( + fixes.expand((e) => e.fixes), + file: Directory.current.file('test', 'goldens', 'ignore_quick_fix.json'), + ); }); test('Emits indented ignore quick-fix', () async { diff --git a/packages/custom_lint/test/run_plugin.dart b/packages/custom_lint/test/run_plugin.dart index 12b11957..b9382ea4 100644 --- a/packages/custom_lint/test/run_plugin.dart +++ b/packages/custom_lint/test/run_plugin.dart @@ -49,6 +49,7 @@ Future startRunnerForApp( bool ignoreErrors = false, bool includeBuiltInLints = true, bool watchMode = false, + bool fix = false, }) async { final zone = Zone.current; final channel = ServerIsolateChannel(); @@ -56,6 +57,7 @@ Future startRunnerForApp( final customLintServer = await CustomLintServer.start( sendPort: channel.receivePort.sendPort, workingDirectory: directory, + fix: fix, delegate: CommandCustomLintDelegate(), includeBuiltInLints: includeBuiltInLints, watchMode: watchMode, diff --git a/packages/custom_lint/test/src/workspace_test.dart b/packages/custom_lint/test/src/workspace_test.dart index 8a3e265e..55446789 100644 --- a/packages/custom_lint/test/src/workspace_test.dart +++ b/packages/custom_lint/test/src/workspace_test.dart @@ -259,7 +259,7 @@ Future createSimpleWorkspace( p.basename(projectEntry), version: Version(1, 0, 0), environment: { - 'sdk': VersionConstraint.parse('>=2.17.0 <4.0.0'), + 'sdk': VersionConstraint.parse('>=3.0.0 <4.0.0'), }, ) else @@ -1168,7 +1168,7 @@ dependency_overrides: group('computePubspec', () { test( - 'If an environment constraint is not specified in a given project, it is considered as "any"', + 'If an environment constraint is not specified in a given project, it is considered as "^3.0.0"', () async { final workingDir = await createSimpleWorkspace([ Pubspec( @@ -1177,9 +1177,7 @@ dependency_overrides: ), Pubspec( 'a', - environment: { - 'sdk': VersionConstraint.parse('>=2.12.0 <3.0.0'), - }, + environment: {'sdk': VersionConstraint.any}, devDependencies: {'plugin1': HostedDependency()}, ), Pubspec( @@ -1201,7 +1199,7 @@ version: 0.0.1 publish_to: 'none' environment: - sdk: ">=2.12.0 <3.0.0" + sdk: ">=3.0.0 <4.0.0" dev_dependencies: plugin1: any @@ -1218,14 +1216,14 @@ dev_dependencies: Pubspec( 'a', environment: { - 'sdk': VersionConstraint.parse('>=2.12.0 <3.0.0'), + 'sdk': VersionConstraint.parse('>=3.12.0 <4.0.0'), }, devDependencies: {'plugin1': HostedDependency()}, ), Pubspec( 'b', environment: { - 'sdk': VersionConstraint.parse('>=2.0.0 <2.19.0'), + 'sdk': VersionConstraint.parse('>=3.0.0 <3.19.0'), }, devDependencies: {'plugin1': HostedDependency()}, ), @@ -1243,7 +1241,7 @@ version: 0.0.1 publish_to: 'none' environment: - sdk: ">=2.12.0 <2.19.0" + sdk: ">=3.12.0 <3.19.0" dev_dependencies: plugin1: any diff --git a/packages/custom_lint/tools/analyzer_plugin/pubspec.yaml b/packages/custom_lint/tools/analyzer_plugin/pubspec.yaml index 8dd245cf..33b1c25e 100644 --- a/packages/custom_lint/tools/analyzer_plugin/pubspec.yaml +++ b/packages/custom_lint/tools/analyzer_plugin/pubspec.yaml @@ -4,7 +4,7 @@ version: 0.5.11 publish_to: none environment: - sdk: ">=2.14.0 <4.0.0" + sdk: ">=3.0.0 <4.0.0" dependencies: custom_lint: 0.5.11 diff --git a/packages/custom_lint_builder/CHANGELOG.md b/packages/custom_lint_builder/CHANGELOG.md index 1798e546..77adca3e 100644 --- a/packages/custom_lint_builder/CHANGELOG.md +++ b/packages/custom_lint_builder/CHANGELOG.md @@ -1,3 +1,8 @@ +## Unreleased patch + +- Bumped minimum Dart SDK to 3.0.0 +- Added support for `--fix` + ## 0.5.14 - 2024-02-03 - `custom_lint_core` upgraded to `0.5.14` diff --git a/packages/custom_lint_builder/example/example_lint/pubspec.yaml b/packages/custom_lint_builder/example/example_lint/pubspec.yaml index 7e22afae..119f47e0 100644 --- a/packages/custom_lint_builder/example/example_lint/pubspec.yaml +++ b/packages/custom_lint_builder/example/example_lint/pubspec.yaml @@ -2,7 +2,7 @@ name: custom_lint_builder_example_lint publish_to: none environment: - sdk: '>=2.16.0 <4.0.0' + sdk: '>=3.0.0 <4.0.0' dependencies: analyzer: ">=5.12.0 <7.0.0" diff --git a/packages/custom_lint_builder/example/pubspec.yaml b/packages/custom_lint_builder/example/pubspec.yaml index dfaefdfa..c8ee79e7 100644 --- a/packages/custom_lint_builder/example/pubspec.yaml +++ b/packages/custom_lint_builder/example/pubspec.yaml @@ -2,7 +2,7 @@ name: custom_lint_builder_example_app publish_to: none environment: - sdk: '>=2.16.0 <4.0.0' + sdk: '>=3.0.0 <4.0.0' dependencies: riverpod: ^2.0.0 diff --git a/packages/custom_lint_builder/lib/src/channel.dart b/packages/custom_lint_builder/lib/src/channel.dart index ba97835f..286635a4 100644 --- a/packages/custom_lint_builder/lib/src/channel.dart +++ b/packages/custom_lint_builder/lib/src/channel.dart @@ -58,6 +58,7 @@ Future runSocket( Map pluginMains, { required int port, required String host, + required bool fix, required bool includeBuiltInLints, }) async { late Future client; @@ -77,6 +78,7 @@ Future runSocket( return CustomLintPluginClient( includeBuiltInLints: includeBuiltInLints, + fix: fix, _SocketCustomLintClientChannel( socketChannel, registeredPlugins, diff --git a/packages/custom_lint_builder/lib/src/client.dart b/packages/custom_lint_builder/lib/src/client.dart index d9cde641..80704490 100644 --- a/packages/custom_lint_builder/lib/src/client.dart +++ b/packages/custom_lint_builder/lib/src/client.dart @@ -16,6 +16,8 @@ import 'package:analyzer/file_system/physical_file_system.dart'; import 'package:analyzer/source/line_info.dart'; import 'package:analyzer/source/source_range.dart'; import 'package:analyzer_plugin/plugin/plugin.dart' as analyzer_plugin; +import 'package:analyzer_plugin/protocol/protocol_common.dart' + as analyzer_plugin; import 'package:analyzer_plugin/protocol/protocol_generated.dart' as analyzer_plugin; import 'package:analyzer_plugin/starter.dart' as analyzer_plugin; @@ -40,6 +42,7 @@ import 'package:meta/meta.dart'; import 'package:package_config/package_config.dart' show PackageConfig; import 'package:path/path.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; +import 'package:rxdart/rxdart.dart'; import 'package:rxdart/subjects.dart'; import '../custom_lint_builder.dart'; @@ -88,12 +91,34 @@ Future _isVmServiceEnabled() async { return serviceInfo.serverUri != null; } +extension on analyzer_plugin.AnalysisErrorFixes { + bool canBatchFix(String filePath) { + final fixesExcludingIgnores = + fixes.where((change) => !change.isIgnoreChange).toList(); + + return fixesExcludingIgnores.length == 1 && + fixesExcludingIgnores.single.canBatchFix(filePath); + } +} + +extension on analyzer_plugin.PrioritizedSourceChange { + bool canBatchFix(String filePath) { + return change.edits.every((element) => element.file == filePath); + } + + bool get isIgnoreChange { + return change.id == IgnoreCode.ignoreForFileCode || + change.id == IgnoreCode.ignoreForLineCode; + } +} + /// The custom_lint client class CustomLintPluginClient { /// The custom_lint client CustomLintPluginClient( this._channel, { required this.includeBuiltInLints, + required this.fix, }) { _analyzerPlugin = _ClientAnalyzerPlugin( _channel, @@ -107,8 +132,13 @@ class CustomLintPluginClient { _channelInputSub = _channel.input.listen(_handleCustomLintRequest); } - /// Whether + /// Whether to include lints automatically by custom_lint. + /// This includes: + /// - Errors at the top of the file when a lint threw final bool includeBuiltInLints; + + /// Whether to attempt fixing analysis issues before reporting them. + final bool fix; late final StreamSubscription _channelInputSub; late final Future _hotReloader; final CustomLintClientChannel _channel; @@ -255,9 +285,15 @@ class _CustomLintAnalysisConfigs { ); final rules = _lintRulesForContext(activePluginsForContext, configs); - final fixes = _fixesForRules(rules); - final assists = - _assistsForContext(activePluginsForContext, configs, client); + final fixes = _fixesForRules( + rules, + includeBuiltInLints: client.includeBuiltInLints, + ); + final assists = _assistsForContext( + activePluginsForContext, + configs, + client, + ); return _CustomLintAnalysisConfigs( configs, @@ -279,12 +315,15 @@ class _CustomLintAnalysisConfigs { .toList(); } - static Map> _fixesForRules(List rules) { + static Map> _fixesForRules( + List rules, { + required bool includeBuiltInLints, + }) { return { for (final rule in rules) rule.code: [ ...rule.getFixes(), - IgnoreCode(), + if (includeBuiltInLints) IgnoreCode(), ], }; } @@ -341,7 +380,7 @@ class _ClientAnalyzerPlugin extends analyzer_plugin.ServerPlugin { var _customLintConfigsForAnalysisContexts = {}; final _analysisErrorsForAnalysisContexts = - <_AnalysisErrorsKey, Set>{}; + <_AnalysisErrorsKey, Iterable>{}; @override List get fileGlobsToAnalyze => ['*']; @@ -535,8 +574,8 @@ class _ClientAnalyzerPlugin extends analyzer_plugin.ServerPlugin { return analyzer_plugin.EditGetFixesResult([]); } - final analysisErrorFixes = await Future.wait([ - for (final error in errorsAtOffset) + final allAnalysisErrorFixes = await Future.wait([ + for (final error in analysisErrorsForContext) _handlesFixesForError( error, analysisErrorsForContext, @@ -546,14 +585,78 @@ class _ClientAnalyzerPlugin extends analyzer_plugin.ServerPlugin { ), ]); - return analyzer_plugin.EditGetFixesResult( - analysisErrorFixes.whereNotNull().toList(), - ); + final analysisErrorFixesForOffset = allAnalysisErrorFixes + .whereNotNull() + .where( + (fix) => + parameters.offset >= fix.error.location.offset && + parameters.offset <= + fix.error.location.offset + fix.error.location.length, + ) + .toList(); + + final fixAll = {}; + for (final fix in allAnalysisErrorFixes) { + if (fix == null) continue; + + final errorCode = fix.error.code; + fixAll.putIfAbsent(errorCode, () { + final analysisErrorsWithCode = allAnalysisErrorFixes + .whereNotNull() + .where((fix) => fix.error.code == errorCode) + .toList(); + + // Don't show "fix-all" unless at least two errors have the same code. + if (analysisErrorsWithCode.length < 2) return null; + + final fixesWithCode = analysisErrorsWithCode + .where((e) => e.canBatchFix(parameters.file)) + // Ignoring "ignore" fixes + .map((e) { + final fixesExcludingIgnores = + e.fixes.where((change) => !change.isIgnoreChange).toList(); + + return (fixes: fixesExcludingIgnores, error: e.error); + }).sorted( + (a, b) => b.error.location.offset - a.error.location.offset, + ); + + // Don't show fix-all if there's no good fix. + if (fixesWithCode.isEmpty) return null; + + final priority = fixesWithCode + .expand((e) => e.fixes) + .map((e) => e.priority - 1) + .firstOrNull ?? + 0; + + return analyzer_plugin.AnalysisErrorFixes( + fix.error, + fixes: [ + analyzer_plugin.PrioritizedSourceChange( + priority, + analyzer_plugin.SourceChange( + 'Fix all "$errorCode"', + edits: fixesWithCode + .expand((e) => e.fixes) + .expand((e) => e.change.edits) + .toList(), + ), + ), + ], + ); + }); + } + + return analyzer_plugin.EditGetFixesResult([ + ...analysisErrorFixesForOffset, + ...fixAll.values.whereNotNull(), + ]); } Future _handlesFixesForError( AnalysisError analysisError, - Set allErrors, + Iterable allErrors, _CustomLintAnalysisConfigs configs, CustomLintResolver resolver, analyzer_plugin.EditGetFixesParams parameters, @@ -767,15 +870,16 @@ class _ClientAnalyzerPlugin extends analyzer_plugin.ServerPlugin { analysisContext.createResolverForFile(resourceProvider.getFile(path)); if (resolver == null) return; - final lints = []; + final lintsBeforeExpectLint = []; final reporterBeforeExpectLint = ErrorReporter( // TODO assert that a LintRule only emits lints with a code matching LintRule.code // TODO asserts lintRules can only emit lints in the analyzed file - _AnalysisErrorListenerDelegate((lint) { - final ignoreForLine = parseIgnoreForLine(lint.offset, resolver); + _AnalysisErrorListenerDelegate((analysisError) async { + final ignoreForLine = + parseIgnoreForLine(analysisError.offset, resolver); - if (!ignoreForLine.isIgnored(lint.errorCode.name)) { - lints.add(lint); + if (!ignoreForLine.isIgnored(analysisError.errorCode.name)) { + lintsBeforeExpectLint.add(analysisError); } }), resolver.source, @@ -830,16 +934,18 @@ class _ClientAnalyzerPlugin extends analyzer_plugin.ServerPlugin { isNonNullableByDefault: false, ); - ExpectLint(lints).run(resolver, analyzerPluginReporter); + ExpectLint(lintsBeforeExpectLint).run(resolver, analyzerPluginReporter); - final key = - _AnalysisErrorsKey(filePath: path, analysisContext: analysisContext); - _analysisErrorsForAnalysisContexts[key] = { - // Combining lints before/after applying expect_error - // This is to enable fixes to access both - ...allAnalysisErrors, - ...lints, - }; + if (await _applyFixes(allAnalysisErrors, resolver, configs, path: path)) { + // Applying fixes re-runs analysis, so lints should've already been sent. + return; + } + + final key = _AnalysisErrorsKey( + filePath: path, + analysisContext: analysisContext, + ); + _analysisErrorsForAnalysisContexts[key] = allAnalysisErrors; _channel.sendEvent( CustomLintEvent.analyzerPluginNotification( @@ -856,6 +962,113 @@ class _ClientAnalyzerPlugin extends analyzer_plugin.ServerPlugin { }); } + Future _applyFixes( + List allAnalysisErrors, + CustomLintResolver resolver, + _CustomLintAnalysisConfigs configs, { + required String path, + }) async { + // The list of already fixed codes, to avoid trying to re-fix a lint + // that should've been fixed before. + final fixedCodes = + (Zone.current[#_fixedCodes] as Set?) ?? {}; + + final allFixes = await _computeFixes( + allAnalysisErrors, + resolver, + configs, + fixedCodes, + path: path, + ).toList(); + + if (allFixes.isEmpty) return false; + + final source = resolver.source.contents.data; + final firstFixCode = allFixes.first.analysisError.errorCode; + final didApplyAllFixes = + allFixes.every((e) => e.analysisError.errorCode == firstFixCode); + + try { + // Apply fixes from top to bottom. + allFixes.sort( + (a, b) => b.analysisError.offset - a.analysisError.offset, + ); + + final editedSource = analyzer_plugin.SourceEdit.applySequence( + source, + // We apply fixes only once at a time, to avoid conflicts. + // To do so, we take the first fixed lint code, and apply fixes + // only for that code. + allFixes + .where((e) => e.analysisError.errorCode == firstFixCode) + .expand((e) => e.edits), + ); + + if (didApplyAllFixes) { + // Apply fixes to the file + io.File(path).writeAsStringSync(editedSource); + } + + // Update in-memory file content before re-running analysis. + resourceProvider.setOverlay( + path, + content: editedSource, + modificationStamp: DateTime.now().millisecondsSinceEpoch, + ); + + // Re-run analysis to recompute lints + return runZoned( + () async { + await contentChanged([path]); + return true; + }, + zoneValues: { + // We update the list of fixed codes to avoid re-fixing the same lint + #_fixedCodes: {...fixedCodes, firstFixCode.name}, + }, + ); + } catch (e) { + // Something failed. We report the original lints + io.stderr.writeln('Failed to apply fixes for $path.\n$e'); + return false; + } + } + + Stream< + ({ + AnalysisError analysisError, + Iterable edits, + })> _computeFixes( + List allAnalysisErrors, + CustomLintResolver resolver, + _CustomLintAnalysisConfigs configs, + Set fixedCodes, { + required String path, + }) async* { + if (!_client.fix) return; + + for (final analysisError in allAnalysisErrors) { + if (fixedCodes.contains(analysisError.errorCode.name)) continue; + + final fixesForLint = await _handlesFixesForError( + analysisError, + allAnalysisErrors.toSet(), + configs, + resolver, + analyzer_plugin.EditGetFixesParams(path, analysisError.offset), + ); + + if (fixesForLint == null || !fixesForLint.canBatchFix(resolver.path)) { + continue; + } + + yield ( + analysisError: analysisError, + edits: fixesForLint.fixes.single.change.edits.expand((e) => e.edits), + ); + } + } + /// Queue an operation to be awaited by [_awaitAnalysisDone] Future _runOperation(FutureOr Function() cb) async { final future = Future(cb); diff --git a/packages/custom_lint_builder/lib/src/ignore.dart b/packages/custom_lint_builder/lib/src/ignore.dart index b382c308..f71dfc4f 100644 --- a/packages/custom_lint_builder/lib/src/ignore.dart +++ b/packages/custom_lint_builder/lib/src/ignore.dart @@ -100,6 +100,12 @@ List parseIgnoreForFile(String source) { /// Built in fix to ignore a lint. class IgnoreCode extends DartFix { + /// The code for 'ignore for line' fix. + static const ignoreForLineCode = 'ignore_for_line'; + + /// The code for 'ignore for file' fix. + static const ignoreForFileCode = 'ignore_for_file'; + @override void run( CustomLintResolver resolver, @@ -113,7 +119,8 @@ class IgnoreCode extends DartFix { final ignoreForLineChangeBuilder = reporter.createChangeBuilder( message: 'Ignore "${analysisError.errorCode.name}" for line', - priority: 35, + priority: 1, + id: ignoreForLineCode, ); ignoreForLineChangeBuilder.addDartFileEdit((builder) { @@ -141,7 +148,8 @@ class IgnoreCode extends DartFix { final ignoreForFileChangeBuilder = reporter.createChangeBuilder( message: 'Ignore "${analysisError.errorCode.name}" for file', - priority: 34, + priority: 0, + id: ignoreForFileCode, ); ignoreForFileChangeBuilder.addDartFileEdit((builder) { diff --git a/packages/custom_lint_builder/pubspec.yaml b/packages/custom_lint_builder/pubspec.yaml index 486cf79b..a77f89cc 100644 --- a/packages/custom_lint_builder/pubspec.yaml +++ b/packages/custom_lint_builder/pubspec.yaml @@ -4,7 +4,7 @@ description: A package to help writing custom linters repository: https://github.com/invertase/dart_custom_lint environment: - sdk: ">=2.19.0 <4.0.0" + sdk: ">=3.0.0 <4.0.0" dependencies: analyzer: ">=5.12.0 <7.0.0" diff --git a/packages/custom_lint_builder/pubspec_overrides.yaml b/packages/custom_lint_builder/pubspec_overrides.yaml index 1e161515..8c6e9638 100644 --- a/packages/custom_lint_builder/pubspec_overrides.yaml +++ b/packages/custom_lint_builder/pubspec_overrides.yaml @@ -2,4 +2,6 @@ dependency_overrides: custom_lint: path: ../custom_lint custom_lint_core: - path: ../custom_lint_core \ No newline at end of file + path: ../custom_lint_core + lint_visitor_generator: + path: ../lint_visitor_generator diff --git a/packages/custom_lint_core/CHANGELOG.md b/packages/custom_lint_core/CHANGELOG.md index 2ae9d361..52cdb5f7 100644 --- a/packages/custom_lint_core/CHANGELOG.md +++ b/packages/custom_lint_core/CHANGELOG.md @@ -1,3 +1,11 @@ +## Unreleased minor + +- Bumped minimum Dart SDK to 3.0.0 +- goldens with diffs now include the priority, ID, selection and file path. +- **breaking**: `encodePrioritizedSourceChanges`/`matcherNormalizedPrioritizedSourceChangeSnapshot`'s `source` + parameter now takes a `Map? sources` instead of `String? source`. + This enables goldens to handle fixes that emit to a different file. + ## 0.5.14 - 2024-02-03 - Improved formatting when specifying `source` on `encodePrioritizedSourceChanges`/`matcherNormalizedPrioritizedSourceChangeSnapshot` diff --git a/packages/custom_lint_core/lib/src/change_reporter.dart b/packages/custom_lint_core/lib/src/change_reporter.dart index f0216e64..3b531fc5 100644 --- a/packages/custom_lint_core/lib/src/change_reporter.dart +++ b/packages/custom_lint_core/lib/src/change_reporter.dart @@ -19,6 +19,7 @@ abstract class ChangeReporter { ChangeBuilder createChangeBuilder({ required String message, required int priority, + String? id, }); } @@ -36,11 +37,13 @@ class ChangeReporterImpl implements ChangeReporter { ChangeBuilder createChangeBuilder({ required String message, required int priority, + String? id, }) { final changeBuilderImpl = _ChangeBuilderImpl( message, analysisSession: _analysisSession, priority: priority, + id: id, path: _resolver.path, ); _changeBuilders.add(changeBuilderImpl); @@ -108,6 +111,7 @@ class _ChangeBuilderImpl implements ChangeBuilder { this._message, { required this.path, required this.priority, + required this.id, required AnalysisSession analysisSession, }) : _innerChangeBuilder = analyzer_plugin.ChangeBuilder(session: analysisSession); @@ -115,6 +119,7 @@ class _ChangeBuilderImpl implements ChangeBuilder { final String _message; final int priority; final String path; + final String? id; final analyzer_plugin.ChangeBuilder _innerChangeBuilder; final _operations = >[]; @@ -163,7 +168,9 @@ class _ChangeBuilderImpl implements ChangeBuilder { return PrioritizedSourceChange( priority, - _innerChangeBuilder.sourceChange..message = _message, + _innerChangeBuilder.sourceChange + ..id = id + ..message = _message, ); } } diff --git a/packages/custom_lint_core/lib/src/matcher.dart b/packages/custom_lint_core/lib/src/matcher.dart index acc5c0ad..0cf3c833 100644 --- a/packages/custom_lint_core/lib/src/matcher.dart +++ b/packages/custom_lint_core/lib/src/matcher.dart @@ -5,108 +5,151 @@ import 'dart:math'; import 'package:analyzer/source/line_info.dart'; import 'package:analyzer_plugin/protocol/protocol_common.dart'; import 'package:analyzer_plugin/protocol/protocol_generated.dart'; +import 'package:collection/collection.dart'; +import 'package:glob/glob.dart'; import 'package:matcher/matcher.dart'; import 'package:meta/meta.dart'; -import 'package:path/path.dart'; +import 'package:path/path.dart' as p; /// Encodes a list of [PrioritizedSourceChange] into a string. /// /// This strips the file paths from the json output. /// -/// If [source] is specified, the changes will be applied on the source, -/// and the result will be inserted in the json output. +/// {@template encodePrioritizedSourceChanges.args} +/// - [sources] is an optional map of file paths to their content. +/// If specified, the changes will be applied to their corresponding source, +/// and the result will be saved in the diff. +/// Glob syntax is supported in the file paths. +/// - [relativePath] can be specified to change file paths in goldens +/// to be relative to a specific directory. +/// {@endtemplate} String encodePrioritizedSourceChanges( - List changes, { + Iterable changes, { JsonEncoder? encoder, - String? source, + Map? sources, + String? relativePath, }) { - if (source != null) { - final sourceLineInfo = LineInfo.fromContent(source); - + if (sources != null) { final buffer = StringBuffer(); for (final prioritizedSourceChange in changes) { buffer.writeln('Message: `${prioritizedSourceChange.change.message}`'); + buffer.writeln('Priority: ${prioritizedSourceChange.priority}'); + if (prioritizedSourceChange.change.id != null) { + buffer.writeln('Id: `${prioritizedSourceChange.change.id}`'); + } + if (prioritizedSourceChange.change.selection case final selection?) { + buffer.writeln( + 'Selection: offset ${selection.offset} ; ' + 'file: `${selection.file}`; ' + 'length: ${prioritizedSourceChange.change.selectionLength}', + ); + } + + final files = prioritizedSourceChange.change.edits + .map((e) => p.normalize(p.relative(e.file, from: relativePath))) + .toSet() + .sortedBy((a) => a); + + for (final file in files) { + final source = sources.entries + .firstWhereOrNull( + (e) => + Glob(e.key).matches(file) || + // workaround to https://github.com/dart-lang/glob/issues/72 + Glob('/${e.key}').matches(file), + ) + ?.value; + if (source == null) { + throw StateError('No source found for file: $file'); + } + + final sourceLineInfo = LineInfo.fromContent(source); - final output = SourceEdit.applySequence( - source, - prioritizedSourceChange.change.edits.expand((element) => element.edits), - ); + final output = SourceEdit.applySequence( + source, + prioritizedSourceChange.change.edits + .expand((element) => element.edits), + ); - final outputLineInfo = LineInfo.fromContent(output); + final outputLineInfo = LineInfo.fromContent(output); - // Get the offset of the first changed character between output and source. - var firstDiffOffset = 0; - for (; firstDiffOffset < source.length; firstDiffOffset++) { - if (source[firstDiffOffset] != output[firstDiffOffset]) { - break; + // Get the offset of the first changed character between output and source. + var firstDiffOffset = 0; + for (; firstDiffOffset < source.length; firstDiffOffset++) { + if (source[firstDiffOffset] != output[firstDiffOffset]) { + break; + } } - } - // Get the last changed character offset between output and source. - var endSourceOffset = source.length - 1; - var endOutputOffset = output.length - 1; - for (; - endOutputOffset > firstDiffOffset && - endSourceOffset > firstDiffOffset; - endOutputOffset--, endSourceOffset--) { - if (source[endSourceOffset] != output[endOutputOffset]) { - break; + // Get the last changed character offset between output and source. + var endSourceOffset = source.length - 1; + var endOutputOffset = output.length - 1; + for (; + endOutputOffset > firstDiffOffset && + endSourceOffset > firstDiffOffset; + endOutputOffset--, endSourceOffset--) { + if (source[endSourceOffset] != output[endOutputOffset]) { + break; + } } - } - final firstChangedLine = - sourceLineInfo.getLocation(firstDiffOffset).lineNumber - 1; - - void writeDiff({ - required String file, - required LineInfo lineInfo, - required int endOffset, - required String token, - required int leadingCount, - required int trailingCount, - }) { - final lastChangedLine = lineInfo.getLocation(endOffset).lineNumber - 1; - final endLine = - min(lastChangedLine + trailingCount, lineInfo.lineCount - 1); - for (var line = max(0, firstChangedLine - leadingCount); - line <= endLine; - line++) { - final changed = line >= firstChangedLine && line <= lastChangedLine; - if (changed) buffer.write(token); - - final endOfSource = !(line + 1 < lineInfo.lineCount); - - buffer.write( - file.substring( - lineInfo.getOffsetOfLine(line), - endOfSource ? null : lineInfo.getOffsetOfLine(line + 1) - 1, - ), - ); - if (!endOfSource) buffer.writeln(); + final firstChangedLine = + sourceLineInfo.getLocation(firstDiffOffset).lineNumber - 1; + + void writeDiff({ + required String file, + required LineInfo lineInfo, + required int endOffset, + required String token, + required int leadingCount, + required int trailingCount, + }) { + final lastChangedLine = + lineInfo.getLocation(endOffset).lineNumber - 1; + final endLine = + min(lastChangedLine + trailingCount, lineInfo.lineCount - 1); + for (var line = max(0, firstChangedLine - leadingCount); + line <= endLine; + line++) { + final changed = line >= firstChangedLine && line <= lastChangedLine; + if (changed) buffer.write(token); + + final endOfSource = !(line + 1 < lineInfo.lineCount); + + buffer.write( + file.substring( + lineInfo.getOffsetOfLine(line), + endOfSource ? null : lineInfo.getOffsetOfLine(line + 1) - 1, + ), + ); + if (!endOfSource) buffer.writeln(); + } } + + buffer.writeln('Diff for file `$file:${firstChangedLine + 1}`:'); + buffer.writeln('```'); + writeDiff( + file: source, + lineInfo: sourceLineInfo, + endOffset: endSourceOffset, + leadingCount: 2, + trailingCount: 0, + token: '- ', + ); + + writeDiff( + file: output, + lineInfo: outputLineInfo, + endOffset: endOutputOffset, + leadingCount: 0, + trailingCount: 2, + token: '+ ', + ); + buffer.writeln('```'); } - buffer.writeln('=== diff (starting at line ${firstChangedLine + 1})'); - writeDiff( - file: source, - lineInfo: sourceLineInfo, - endOffset: endSourceOffset, - leadingCount: 2, - trailingCount: 0, - token: '- ', - ); - - writeDiff( - file: output, - lineInfo: outputLineInfo, - endOffset: endOutputOffset, - leadingCount: 0, - trailingCount: 2, - token: '+ ', - ); - - buffer.writeln('==='); + buffer.writeln('---'); } return buffer.toString(); @@ -131,18 +174,19 @@ String encodePrioritizedSourceChanges( /// This effectively encode the list of changes, remove file paths from the result, /// and compare this output with the content of a file. /// -/// If [source] is specified, the changes will be applied on the source, -/// and the result will be inserted in the json output. +/// {@macro encodePrioritizedSourceChanges.args} @visibleForTesting Matcher matcherNormalizedPrioritizedSourceChangeSnapshot( String filePath, { JsonEncoder? encoder, - String? source, + Map? sources, + String? relativePath, }) { return _MatcherNormalizedPrioritizedSourceChangeSnapshot( filePath, encoder: encoder, - source: source, + sources: sources, + relativePath: relativePath, ); } @@ -150,23 +194,25 @@ class _MatcherNormalizedPrioritizedSourceChangeSnapshot extends Matcher { _MatcherNormalizedPrioritizedSourceChangeSnapshot( this.path, { this.encoder, - String? source, - }) : _source = source; + this.sources, + this.relativePath, + }); final String path; final JsonEncoder? encoder; - final String? _source; + final Map? sources; + final String? relativePath; static final Object _mismatchedValueKey = Object(); static final Object _expectedKey = Object(); @override bool matches( - covariant List object, + covariant Iterable object, Map matchState, ) { - final file = isRelative(path) - ? File(join(Directory.current.path, 'test', path)) + final file = p.isRelative(path) + ? File(p.join(Directory.current.path, 'test', path)) : File(path); if (!file.existsSync()) { matchState[_mismatchedValueKey] = 'File not found: $path'; @@ -176,7 +222,8 @@ class _MatcherNormalizedPrioritizedSourceChangeSnapshot extends Matcher { final actual = encodePrioritizedSourceChanges( object, encoder: encoder, - source: _source, + sources: sources, + relativePath: relativePath, ); final expected = file.readAsStringSync(); diff --git a/packages/custom_lint_core/pubspec.yaml b/packages/custom_lint_core/pubspec.yaml index 0495e077..b9c18552 100644 --- a/packages/custom_lint_core/pubspec.yaml +++ b/packages/custom_lint_core/pubspec.yaml @@ -4,7 +4,7 @@ description: A package to help writing custom linters repository: https://github.com/invertase/dart_custom_lint environment: - sdk: ">=2.19.0 <4.0.0" + sdk: ">=3.0.0 <4.0.0" dependencies: analyzer: ">=5.12.0 <7.0.0" @@ -13,6 +13,7 @@ dependencies: # Using tight constraints as custom_lint_builder communicate with each-other # using a specific contract custom_lint: 0.5.11 + glob: ^2.1.2 matcher: ^0.12.0 meta: ^1.7.0 package_config: ^2.1.0 diff --git a/packages/custom_lint_core/test/assist_test.dart b/packages/custom_lint_core/test/assist_test.dart index 9c1a90a0..013b821b 100644 --- a/packages/custom_lint_core/test/assist_test.dart +++ b/packages/custom_lint_core/test/assist_test.dart @@ -10,14 +10,14 @@ import 'package:custom_lint_core/src/change_reporter.dart'; import 'package:custom_lint_core/src/lint_rule.dart'; import 'package:custom_lint_core/src/matcher.dart'; import 'package:custom_lint_core/src/resolver.dart'; -import 'package:path/path.dart'; +import 'package:path/path.dart' as p; import 'package:test/test.dart'; File writeToTemporaryFile(String content) { final tempDir = io.Directory.systemTemp.createTempSync(); addTearDown(() => tempDir.deleteSync(recursive: true)); - final file = io.File(join(tempDir.path, 'file.dart')) + final file = io.File(p.join(tempDir.path, 'file.dart')) ..createSync(recursive: true) ..writeAsStringSync(content); @@ -45,7 +45,8 @@ void main() { await changes, matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ); expect( @@ -53,7 +54,8 @@ void main() { isNot( matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot2.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ), ); @@ -63,7 +65,8 @@ void main() { isNot( matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ), ); @@ -71,7 +74,8 @@ void main() { await changes2, matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot2.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ); }); @@ -138,7 +142,8 @@ void main() { await changes, matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ); expect( @@ -146,7 +151,8 @@ void main() { isNot( matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot2.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ), ); @@ -156,7 +162,8 @@ void main() { isNot( matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ), ); @@ -164,7 +171,8 @@ void main() { await changes2, matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot2.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ); }); diff --git a/packages/custom_lint_core/test/fix_test.dart b/packages/custom_lint_core/test/fix_test.dart index b025bf5b..e3c20e05 100644 --- a/packages/custom_lint_core/test/fix_test.dart +++ b/packages/custom_lint_core/test/fix_test.dart @@ -34,7 +34,8 @@ void main() { await changes, matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ); expect( @@ -42,7 +43,8 @@ void main() { isNot( matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot2.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ), ); @@ -52,7 +54,8 @@ void main() { isNot( matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ), ); @@ -60,7 +63,8 @@ void main() { await changes2, matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot2.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ); }); @@ -82,7 +86,8 @@ void main() { await changes, matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ); expect( @@ -90,7 +95,8 @@ void main() { isNot( matcherNormalizedPrioritizedSourceChangeSnapshot( 'snapshot2.diff', - source: fileSource, + sources: {'**/*': fileSource}, + relativePath: file.parent.path, ), ), ); diff --git a/packages/custom_lint_core/test/snapshot.diff b/packages/custom_lint_core/test/snapshot.diff index f4af251c..744a9680 100644 --- a/packages/custom_lint_core/test/snapshot.diff +++ b/packages/custom_lint_core/test/snapshot.diff @@ -1,7 +1,10 @@ Message: `MyAssist` -=== diff (starting at line 2) +Priority: 1 +Diff for file `file.dart:2`: +``` void main() { - print('Hello world'); + Helloprint('Hello world'); } -=== +``` +--- diff --git a/packages/custom_lint_core/test/snapshot2.diff b/packages/custom_lint_core/test/snapshot2.diff index b0696eb3..21299764 100644 --- a/packages/custom_lint_core/test/snapshot2.diff +++ b/packages/custom_lint_core/test/snapshot2.diff @@ -1,7 +1,10 @@ Message: `Another` -=== diff (starting at line 2) +Priority: 1 +Diff for file `file.dart:2`: +``` void main() { - print('Hello world'); + Helloprint('Hello world'); } -=== +``` +--- diff --git a/packages/custom_lint_core/test/type_checker_test.dart b/packages/custom_lint_core/test/type_checker_test.dart index 7fdebe76..438dc12d 100644 --- a/packages/custom_lint_core/test/type_checker_test.dart +++ b/packages/custom_lint_core/test/type_checker_test.dart @@ -53,7 +53,7 @@ version: 0.2.8 description: A package to help writing custom linters repository: https://github.com/invertase/dart_custom_lint environment: - sdk: ">=2.17.1 <3.0.0" + sdk: ">=3.0.0 <4.0.0" '''); await Process.run( diff --git a/packages/lint_visitor_generator/pubspec.yaml b/packages/lint_visitor_generator/pubspec.yaml index 781cd1e2..a60d3ae7 100644 --- a/packages/lint_visitor_generator/pubspec.yaml +++ b/packages/lint_visitor_generator/pubspec.yaml @@ -2,7 +2,7 @@ name: lint_visitor_generator publish_to: none environment: - sdk: ">=2.17.0 <4.0.0" + sdk: ">=3.0.0 <4.0.0" dependencies: analyzer: ">=5.12.0 <7.0.0" diff --git a/pubspec.yaml b/pubspec.yaml index 79f43313..d10df1bd 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -2,6 +2,6 @@ name: melos_root publish_to: none environment: - sdk: ">=2.17.0 <4.0.0" + sdk: ">=3.0.0 <4.0.0" dev_dependencies: melos: ^3.0.0