diff --git a/pkg/analysis_server/lib/protocol/protocol_generated.dart b/pkg/analysis_server/lib/protocol/protocol_generated.dart index aa832740f63..947615a643c 100644 --- a/pkg/analysis_server/lib/protocol/protocol_generated.dart +++ b/pkg/analysis_server/lib/protocol/protocol_generated.dart @@ -6179,19 +6179,25 @@ class EditDartfixParams implements RequestParams { /** * A list of the files and directories for which edits should be suggested. + * + * If a request is made with path that is invalid, e.g. is not absolute and + * normalized, an error of type INVALID_FILE_PATH_FORMAT will be generated. * If a request is made for a file which does not exist, or which is not * currently subject to analysis (e.g. because it is not associated with any * analysis root specified to analysis.setAnalysisRoots), an error of type - * FORMAT_INVALID_FILE will be generated. + * FILE_NOT_ANALYZED will be generated. */ List get included => _included; /** * A list of the files and directories for which edits should be suggested. + * + * If a request is made with path that is invalid, e.g. is not absolute and + * normalized, an error of type INVALID_FILE_PATH_FORMAT will be generated. * If a request is made for a file which does not exist, or which is not * currently subject to analysis (e.g. because it is not associated with any * analysis root specified to analysis.setAnalysisRoots), an error of type - * FORMAT_INVALID_FILE will be generated. + * FILE_NOT_ANALYZED will be generated. */ void set included(List value) { assert(value != null); diff --git a/pkg/analysis_server/lib/src/edit/edit_dartfix.dart b/pkg/analysis_server/lib/src/edit/edit_dartfix.dart index 99397ab6bae..336cedde79e 100644 --- a/pkg/analysis_server/lib/src/edit/edit_dartfix.dart +++ b/pkg/analysis_server/lib/src/edit/edit_dartfix.dart @@ -3,13 +3,227 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analysis_server/protocol/protocol.dart'; +import 'package:analysis_server/protocol/protocol_generated.dart'; +import 'package:analysis_server/src/analysis_server.dart'; +import 'package:analyzer/analyzer.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/file_system/file_system.dart'; +import 'package:analyzer/src/dart/analysis/driver.dart'; +import 'package:analyzer/src/lint/linter.dart'; +import 'package:analyzer/src/lint/linter_visitor.dart'; +import 'package:analyzer/src/lint/registry.dart'; +import 'package:analyzer/src/services/lint.dart'; +import 'package:front_end/src/base/source.dart'; +import 'package:front_end/src/scanner/token.dart'; +import 'package:source_span/src/span.dart'; class EditDartFix { + final AnalysisServer server; final Request request; + final fixFolders = []; + final fixFiles = []; - EditDartFix(this.request); + EditDartFix(this.server, this.request); Future compute() async { - return new Response.formatInvalidFile(request); + final params = new EditDartfixParams.fromRequest(request); + + // Validate each included file and directory. + final resourceProvider = server.resourceProvider; + final contextManager = server.contextManager; + for (String path in params.included) { + if (!server.isValidFilePath(path)) { + return new Response.invalidFilePathFormat(request, path); + } + Resource res = resourceProvider.getResource(path); + if (!res.exists || + !(contextManager.includedPaths.contains(path) || + contextManager.isInAnalysisRoot(path))) { + return new Response.fileNotAnalyzed(request, path); + } + if (res is Folder) { + fixFolders.add(res); + } else { + fixFiles.add(res); + } + } + + // Get the desired lints + final LintRule preferMixin = Registry.ruleRegistry['prefer_mixin']; + if (preferMixin == null) { + return new Response.serverError( + request, 'Missing PreferMixin lint', null); + } + final preferMixinFix = new PreferMixinFix(this); + preferMixin.reporter = preferMixinFix; + + // Setup + final linters = [ + preferMixin, + ]; + final fixes = [ + preferMixinFix, + ]; + final visitors = []; + final registry = new NodeLintRegistry(false); + for (Linter linter in linters) { + final visitor = linter.getVisitor(); + if (visitor != null) { + visitors.add(visitor); + } + if (linter is NodeLintRule) { + (linter as NodeLintRule).registerNodeProcessors(registry); + } + } + final AstVisitor astVisitor = visitors.isNotEmpty + ? new ExceptionHandlingDelegatingAstVisitor( + visitors, ExceptionHandlingDelegatingAstVisitor.logException) + : null; + final AstVisitor linterVisitor = new LinterVisitor( + registry, ExceptionHandlingDelegatingAstVisitor.logException); + + // TODO(danrubel): Determine if a lint is configured to run as part of + // standard analysis and use those results if available instead of + // running the lint again. + + // Analyze each source file. + final resources = []; + for (String rootPath in contextManager.includedPaths) { + resources.add(resourceProvider.getResource(rootPath)); + } + while (resources.isNotEmpty) { + Resource res = resources.removeLast(); + if (res is Folder) { + for (Resource child in res.getChildren()) { + if (!child.shortName.startsWith('.') && + contextManager.isInAnalysisRoot(child.path)) { + resources.add(child); + } + } + continue; + } + AnalysisResult result = await server.getAnalysisResult(res.path); + CompilationUnit unit = result?.unit; + if (unit != null) { + Source source = result.sourceFactory.forUri2(result.uri); + for (Linter linter in linters) { + linter.reporter.source = source; + } + if (astVisitor != null) { + unit.accept(astVisitor); + } + unit.accept(linterVisitor); + } + } + + // Cleanup + for (Linter linter in linters) { + linter.reporter = null; + } + + // Reporting + final descriptions = []; + for (LinterFix fix in fixes) { + fix.updateResponse(descriptions); + } + return new EditDartfixResult(descriptions, []).toResponse(request.id); + } + + /// Return `true` if the path in within the set of `included` files + /// or is within an `included` directory. + bool isIncluded(String path) { + if (path != null) { + for (File file in fixFiles) { + if (file.path == path) { + return true; + } + } + for (Folder folder in fixFolders) { + if (folder.contains(path)) { + return true; + } + } + } + return false; + } +} + +abstract class LinterFix implements ErrorReporter { + final EditDartFix dartFix; + + @override + Source source; + + LinterFix(this.dartFix); + + @override + void reportError(AnalysisError error) { + // ignored + } + + @override + void reportErrorForElement(ErrorCode errorCode, Element element, + [List arguments]) { + // ignored + } + + @override + void reportErrorForNode(ErrorCode errorCode, AstNode node, + [List arguments]) { + // ignored + } + + @override + void reportErrorForOffset(ErrorCode errorCode, int offset, int length, + [List arguments]) { + // ignored + } + + @override + void reportErrorForSpan(ErrorCode errorCode, SourceSpan span, + [List arguments]) { + // ignored + } + + @override + void reportErrorForToken(ErrorCode errorCode, Token token, + [List arguments]) { + // ignored + } + + @override + void reportTypeErrorForNode( + ErrorCode errorCode, AstNode node, List arguments) { + // ignored + } + + void updateResponse(List descriptions); +} + +class PreferMixinFix extends LinterFix { + final classesToConvert = new Set(); + + PreferMixinFix(EditDartFix dartFix) : super(dartFix); + + @override + void reportErrorForNode(ErrorCode errorCode, AstNode node, + [List arguments]) { + TypeName type = node; + Element element = type.name.staticElement; + String path = element.source?.fullName; + if (dartFix.isIncluded(path)) { + // Only report classes that are `included` + classesToConvert.add(element); + } + } + + @override + void updateResponse(List descriptions) { + final sorted = classesToConvert.toList() + ..sort((c1, c2) => c1.name.compareTo(c2.name)); + for (Element elem in sorted) { + descriptions.add('Convert class to mixin: ${elem.name}'); + } } } diff --git a/pkg/analysis_server/lib/src/edit/edit_domain.dart b/pkg/analysis_server/lib/src/edit/edit_domain.dart index 66c7323eea7..59003e7540b 100644 --- a/pkg/analysis_server/lib/src/edit/edit_domain.dart +++ b/pkg/analysis_server/lib/src/edit/edit_domain.dart @@ -262,15 +262,12 @@ class EditDomainHandler extends AbstractRequestHandler { } Future dartfix(Request request) async { - // TODO(danrubel): Fix only the included sources - //EditDartfixParams params = new EditDartfixParams.fromRequest(request); - // TODO(danrubel): Add support for dartfix plugins // // Compute fixes // - var dartFix = new EditDartFix(request); + var dartFix = new EditDartFix(server, request); Response response = await dartFix.compute(); server.sendResponse(response); diff --git a/pkg/analysis_server/test/integration/support/integration_test_methods.dart b/pkg/analysis_server/test/integration/support/integration_test_methods.dart index 0d95b2b8a13..9d91f7514ff 100644 --- a/pkg/analysis_server/test/integration/support/integration_test_methods.dart +++ b/pkg/analysis_server/test/integration/support/integration_test_methods.dart @@ -1505,10 +1505,13 @@ abstract class IntegrationTestMixin { * included: List * * A list of the files and directories for which edits should be suggested. + * + * If a request is made with path that is invalid, e.g. is not absolute and + * normalized, an error of type INVALID_FILE_PATH_FORMAT will be generated. * If a request is made for a file which does not exist, or which is not * currently subject to analysis (e.g. because it is not associated with * any analysis root specified to analysis.setAnalysisRoots), an error of - * type FORMAT_INVALID_FILE will be generated. + * type FILE_NOT_ANALYZED will be generated. * * Returns * diff --git a/pkg/analysis_server/tool/spec/generated/java/AnalysisServer.java b/pkg/analysis_server/tool/spec/generated/java/AnalysisServer.java index 551d5dddc5c..b01778110d1 100644 --- a/pkg/analysis_server/tool/spec/generated/java/AnalysisServer.java +++ b/pkg/analysis_server/tool/spec/generated/java/AnalysisServer.java @@ -413,9 +413,11 @@ public interface AnalysisServer { * if a change in a specified source requires it. * * @param included A list of the files and directories for which edits should be suggested. If a - * request is made for a file which does not exist, or which is not currently subject to - * analysis (e.g. because it is not associated with any analysis root specified to - * analysis.setAnalysisRoots), an error of type FORMAT_INVALID_FILE will be generated. + * request is made with path that is invalid, e.g. is not absolute and normalized, an error + * of type INVALID_FILE_PATH_FORMAT will be generated. If a request is made for a file + * which does not exist, or which is not currently subject to analysis (e.g. because it is + * not associated with any analysis root specified to analysis.setAnalysisRoots), an error + * of type FILE_NOT_ANALYZED will be generated. */ public void edit_dartfix(List included, DartfixConsumer consumer); diff --git a/pkg/analysis_server/tool/spec/spec_input.html b/pkg/analysis_server/tool/spec/spec_input.html index d5b06238017..7469fc5e767 100644 --- a/pkg/analysis_server/tool/spec/spec_input.html +++ b/pkg/analysis_server/tool/spec/spec_input.html @@ -1947,10 +1947,13 @@

A list of the files and directories for which edits should be suggested. - If a request is made for a file which does not exist, or which is not - currently subject to analysis (e.g. because it is not associated with - any analysis root specified to analysis.setAnalysisRoots), an error of - type FORMAT_INVALID_FILE will be generated. +

+

+ If a request is made with path that is invalid, e.g. is not absolute and normalized, + an error of type INVALID_FILE_PATH_FORMAT will be generated. + If a request is made for a file which does not exist, or which is not currently subject to analysis + (e.g. because it is not associated with any analysis root specified to analysis.setAnalysisRoots), + an error of type FILE_NOT_ANALYZED will be generated.

diff --git a/pkg/analyzer_cli/lib/src/fix/driver.dart b/pkg/analyzer_cli/lib/src/fix/driver.dart index ac43e4c3895..1e022a72ad2 100644 --- a/pkg/analyzer_cli/lib/src/fix/driver.dart +++ b/pkg/analyzer_cli/lib/src/fix/driver.dart @@ -18,6 +18,7 @@ class Driver { Completer serverConnected; Completer analysisComplete; + bool processAnalysisErrors; int errorCount; bool verbose; @@ -64,6 +65,7 @@ class Driver { Future performAnalysis(Options options) async { analysisComplete = new Completer(); + processAnalysisErrors = true; errorCount = 0; outSink.writeln('Analyzing...'); verboseOut('Setup analysis'); @@ -76,6 +78,8 @@ class Driver { const [], ).toJson()); await analysisComplete.future; + analysisComplete = null; + processAnalysisErrors = false; verboseOut('Analysis complete.'); if (errorCount > 0) { // TODO: Ask "Do you want to continue given # of errors?" @@ -84,15 +88,28 @@ class Driver { Future requestFixes(Options options) async { outSink.writeln('Calculating fixes...'); - Map response = await server.send(EDIT_REQUEST_DARTFIX, + Map json = await server.send(EDIT_REQUEST_DARTFIX, new EditDartfixParams(options.analysisRoots).toJson()); - print(response); + ResponseDecoder decoder = new ResponseDecoder(null); + final result = EditDartfixResult.fromJson(decoder, 'result', json); + if (result.description.isNotEmpty) { + outSink.writeln('Recommended changes:'); + for (String line in result.description) { + outSink.writeln(line); + } + } else { + outSink.writeln('No recommended changes.'); + } } Future stopServer(Server server) async { verboseOut('Stopping...'); - await server.send(SERVER_REQUEST_SHUTDOWN, null); - await server.exitCode.timeout(const Duration(seconds: 5), onTimeout: () { + const timeout = const Duration(seconds: 5); + await server.send(SERVER_REQUEST_SHUTDOWN, null).timeout(timeout, + onTimeout: () { + // fall through to wait for exit. + }); + await server.exitCode.timeout(timeout, onTimeout: () { return server.kill('server failed to exit'); }); } @@ -108,11 +125,10 @@ class Driver { onServerConnected( new ServerConnectedParams.fromJson(decoder, 'params', params)); break; -// case SERVER_NOTIFICATION_ERROR: -// outOfTestExpect(params, isServerErrorParams); -// _onServerError -// .add(new ServerErrorParams.fromJson(decoder, 'params', params)); -// break; + case SERVER_NOTIFICATION_ERROR: + onServerError( + new ServerErrorParams.fromJson(decoder, 'params', params)); + break; case SERVER_NOTIFICATION_STATUS: onServerStatus( new ServerStatusParams.fromJson(decoder, 'params', params)); @@ -128,8 +144,10 @@ class Driver { // decoder, 'params', params)); // break; case ANALYSIS_NOTIFICATION_ERRORS: - onAnalysisErrors( - new AnalysisErrorsParams.fromJson(decoder, 'params', params)); + if (processAnalysisErrors) { + onAnalysisErrors( + new AnalysisErrorsParams.fromJson(decoder, 'params', params)); + } break; // case ANALYSIS_NOTIFICATION_FLUSH_RESULTS: // outOfTestExpect(params, isAnalysisFlushResultsParams); @@ -222,10 +240,23 @@ class Driver { serverConnected.complete(); } + void onServerError(ServerErrorParams params) async { + try { + await stopServer(server); + } catch (e) { + // ignored + } + final message = new StringBuffer('Server Error: ')..writeln(params.message); + if (params.stackTrace != null) { + message.writeln(params.stackTrace); + } + printAndFail(message.toString()); + } + void onServerStatus(ServerStatusParams params) { if (params.analysis != null && !params.analysis.isAnalyzing) { verboseOut('Analysis complete'); - analysisComplete.complete(); + analysisComplete?.complete(); } }