From 7b75bd8ece701df3519045162cab83afbc44556d Mon Sep 17 00:00:00 2001 From: Jens Johansen Date: Thu, 5 Nov 2020 09:43:23 +0000 Subject: [PATCH] [CFE] Special case StrongModeNNBDPackageOptOut message in IncrementalCompiler This CL fixes the wrong message, so - when in the process of fixing the package nnbd settings - one gets the correct message, mentioning the right packages and no longer mentioning the fixed ones. Change-Id: Ied26094055e56b44dfdd2e942b98eb9e1e7968fb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170423 Commit-Queue: Jens Johansen Reviewed-by: Johnni Winther --- .../lib/src/messages/codes.dart | 13 +- .../lib/src/messages/diagnostic_message.dart | 2 + .../lib/src/fasta/incremental_compiler.dart | 31 ++++- .../lib/src/fasta/source/source_loader.dart | 120 ++++++++++-------- pkg/front_end/test/messages_json_test.dart | 16 ++- ...id_of_nnbd_issue_error.yaml.world.3.expect | 1 - pkg/kernel/problems.md | 2 + 7 files changed, 122 insertions(+), 63 deletions(-) diff --git a/pkg/_fe_analyzer_shared/lib/src/messages/codes.dart b/pkg/_fe_analyzer_shared/lib/src/messages/codes.dart index bc0393d3c8c..5d95d3b6c75 100644 --- a/pkg/_fe_analyzer_shared/lib/src/messages/codes.dart +++ b/pkg/_fe_analyzer_shared/lib/src/messages/codes.dart @@ -183,6 +183,8 @@ class FormattedMessage implements DiagnosticMessage { Code get code => locatedMessage.code; + String get codeName => code.name; + String get message => locatedMessage.message; String get tip => locatedMessage.tip; @@ -219,6 +221,7 @@ class FormattedMessage implements DiagnosticMessage { "severity": severity.index, "uri": uri?.toString(), "involvedFiles": involvedFiles?.map((u) => u.toString())?.toList(), + "codeName": code.name, }; } @@ -242,8 +245,10 @@ class DiagnosticMessageFromJson implements DiagnosticMessage { final List involvedFiles; + final String codeName; + DiagnosticMessageFromJson(this.ansiFormatted, this.plainTextFormatted, - this.severity, this.uri, this.involvedFiles); + this.severity, this.uri, this.involvedFiles, this.codeName); factory DiagnosticMessageFromJson.fromJson(String jsonString) { Map decoded = json.decode(jsonString); @@ -258,9 +263,10 @@ class DiagnosticMessageFromJson implements DiagnosticMessage { : new List.from(decoded["involvedFiles"]) .map((e) => Uri.parse(e)) .toList(); + String codeName = decoded["codeName"]; - return new DiagnosticMessageFromJson( - ansiFormatted, plainTextFormatted, severity, uri, involvedFiles); + return new DiagnosticMessageFromJson(ansiFormatted, plainTextFormatted, + severity, uri, involvedFiles, codeName); } Map toJson() { @@ -271,6 +277,7 @@ class DiagnosticMessageFromJson implements DiagnosticMessage { "severity": severity.index, "uri": uri?.toString(), "involvedFiles": involvedFiles?.map((u) => u.toString())?.toList(), + "codeName": codeName, }; } diff --git a/pkg/_fe_analyzer_shared/lib/src/messages/diagnostic_message.dart b/pkg/_fe_analyzer_shared/lib/src/messages/diagnostic_message.dart index 3d83aa9ae1c..71462582d97 100644 --- a/pkg/_fe_analyzer_shared/lib/src/messages/diagnostic_message.dart +++ b/pkg/_fe_analyzer_shared/lib/src/messages/diagnostic_message.dart @@ -36,6 +36,8 @@ abstract class DiagnosticMessage { Severity get severity; Iterable get involvedFiles; + + String get codeName; } /// This method is subject to change. diff --git a/pkg/front_end/lib/src/fasta/incremental_compiler.dart b/pkg/front_end/lib/src/fasta/incremental_compiler.dart index 7af91d235b8..906d131921e 100644 --- a/pkg/front_end/lib/src/fasta/incremental_compiler.dart +++ b/pkg/front_end/lib/src/fasta/incremental_compiler.dart @@ -8,6 +8,7 @@ import 'package:front_end/src/api_prototype/experimental_flags.dart'; import 'package:front_end/src/api_prototype/front_end.dart'; import 'package:front_end/src/base/nnbd_mode.dart'; import 'package:front_end/src/fasta/fasta_codes.dart'; +import 'package:front_end/src/fasta/source/source_loader.dart'; import 'package:kernel/binary/ast_from_binary.dart' show BinaryBuilderWithMetadata, @@ -1379,15 +1380,41 @@ class IncrementalCompiler implements IncrementalKernelGenerator { } // Report old problems that wasn't reported again. - for (List messages - in remainingComponentProblems.values) { + Set strongModeNNBDPackageOptOutUris; + for (MapEntry> entry + in remainingComponentProblems.entries) { + List messages = entry.value; for (int i = 0; i < messages.length; i++) { DiagnosticMessageFromJson message = messages[i]; + if (message.codeName == "StrongModeNNBDPackageOptOut") { + // Special case this: Don't issue them here; instead collect them + // to get their uris and re-issue a new error. + strongModeNNBDPackageOptOutUris ??= {}; + strongModeNNBDPackageOptOutUris.add(entry.key); + continue; + } if (issuedProblems.add(message.toJsonString())) { context.options.reportDiagnosticMessage(message); } } } + if (strongModeNNBDPackageOptOutUris != null) { + // Get the builders for these uris; then call + // `SourceLoader.giveCombinedErrorForNonStrongLibraries` on them to issue + // a new error. + Set builders = {}; + SourceLoader loader = userCode.loader; + for (LibraryBuilder builder in loader.builders.values) { + if (strongModeNNBDPackageOptOutUris.contains(builder.fileUri)) { + builders.add(builder); + } + } + FormattedMessage message = loader.giveCombinedErrorForNonStrongLibraries( + builders, + emitNonPackageErrors: false); + issuedProblems.add(message.toJsonString()); + // The problem was issued by the call so don't re-issue it here. + } // Save any new component-problems. _addProblemsAsJsonToRemainingProblems(componentWithDill?.problemsAsJson); diff --git a/pkg/front_end/lib/src/fasta/source/source_loader.dart b/pkg/front_end/lib/src/fasta/source/source_loader.dart index 49d78f47528..e5c901078c5 100644 --- a/pkg/front_end/lib/src/fasta/source/source_loader.dart +++ b/pkg/front_end/lib/src/fasta/source/source_loader.dart @@ -355,61 +355,9 @@ class SourceLoader extends Loader { // config we include each library uri in the message. For non-package // libraries with no corresponding package config we generate a message // per library. - Map> libraryByPackage = {}; - Map enableNonNullableVersionByPackage = {}; - for (LibraryBuilder libraryBuilder in _strongOptOutLibraries) { - Package package = - target.uriTranslator.getPackage(libraryBuilder.importUri); - - if (package != null && - package.languageVersion != null && - package.languageVersion is! InvalidLanguageVersion) { - Version enableNonNullableVersion = - enableNonNullableVersionByPackage[package] ??= - target.getExperimentEnabledVersionInLibrary( - ExperimentalFlag.nonNullable, - new Uri(scheme: 'package', path: package.name)); - Version version = new Version( - package.languageVersion.major, package.languageVersion.minor); - if (version < enableNonNullableVersion) { - (libraryByPackage[package?.name] ??= []).add(libraryBuilder); - continue; - } - } - if (libraryBuilder.importUri.scheme == 'package') { - (libraryByPackage[null] ??= []).add(libraryBuilder); - } else { - // Emit a message that doesn't mention running 'pub'. - addProblem(messageStrongModeNNBDButOptOut, -1, noLength, - libraryBuilder.fileUri); - } - } - if (libraryByPackage.isNotEmpty) { - List involvedFiles = []; - List dependencies = []; - libraryByPackage.forEach((String name, List libraries) { - if (name != null) { - dependencies.add('package:$name'); - for (LibraryBuilder libraryBuilder in libraries) { - involvedFiles.add(libraryBuilder.fileUri); - } - } else { - for (LibraryBuilder libraryBuilder in libraries) { - dependencies.add(libraryBuilder.importUri.toString()); - involvedFiles.add(libraryBuilder.fileUri); - } - } - }); - // Emit a message that suggests to run 'pub' to check for opted in - // versions of the packages. - addProblem( - templateStrongModeNNBDPackageOptOut.withArguments(dependencies), - -1, - -1, - null, - involvedFiles: involvedFiles); - _strongOptOutLibraries = null; - } + giveCombinedErrorForNonStrongLibraries(_strongOptOutLibraries, + emitNonPackageErrors: true); + _strongOptOutLibraries = null; } if (_nnbdMismatchLibraries != null) { for (MapEntry entry @@ -420,6 +368,68 @@ class SourceLoader extends Loader { } } + FormattedMessage giveCombinedErrorForNonStrongLibraries( + Set libraries, + {bool emitNonPackageErrors}) { + Map> libraryByPackage = {}; + Map enableNonNullableVersionByPackage = {}; + for (LibraryBuilder libraryBuilder in libraries) { + final Package package = + target.uriTranslator.getPackage(libraryBuilder.importUri); + + if (package != null && + package.languageVersion != null && + package.languageVersion is! InvalidLanguageVersion) { + Version enableNonNullableVersion = + enableNonNullableVersionByPackage[package] ??= + target.getExperimentEnabledVersionInLibrary( + ExperimentalFlag.nonNullable, + new Uri(scheme: 'package', path: package.name)); + Version version = new Version( + package.languageVersion.major, package.languageVersion.minor); + if (version < enableNonNullableVersion) { + (libraryByPackage[package.name] ??= []).add(libraryBuilder); + continue; + } + } + if (libraryBuilder.importUri.scheme == 'package') { + (libraryByPackage[null] ??= []).add(libraryBuilder); + } else { + if (emitNonPackageErrors) { + // Emit a message that doesn't mention running 'pub'. + addProblem(messageStrongModeNNBDButOptOut, -1, noLength, + libraryBuilder.fileUri); + } + } + } + if (libraryByPackage.isNotEmpty) { + List involvedFiles = []; + List dependencies = []; + libraryByPackage.forEach((String name, List libraries) { + if (name != null) { + dependencies.add('package:$name'); + for (LibraryBuilder libraryBuilder in libraries) { + involvedFiles.add(libraryBuilder.fileUri); + } + } else { + for (LibraryBuilder libraryBuilder in libraries) { + dependencies.add(libraryBuilder.importUri.toString()); + involvedFiles.add(libraryBuilder.fileUri); + } + } + }); + // Emit a message that suggests to run 'pub' to check for opted in + // versions of the packages. + return addProblem( + templateStrongModeNNBDPackageOptOut.withArguments(dependencies), + -1, + -1, + null, + involvedFiles: involvedFiles); + } + return null; + } + List getSource(List bytes) { // bytes is 0-terminated. We don't want that included. if (bytes is Uint8List) { diff --git a/pkg/front_end/test/messages_json_test.dart b/pkg/front_end/test/messages_json_test.dart index 4e84e65b9cb..e0f5620c51b 100644 --- a/pkg/front_end/test/messages_json_test.dart +++ b/pkg/front_end/test/messages_json_test.dart @@ -8,15 +8,22 @@ import 'package:_fe_analyzer_shared/src/messages/diagnostic_message.dart' import 'package:_fe_analyzer_shared/src/messages/severity.dart' show Severity; import 'package:front_end/src/fasta/fasta_codes.dart' - show DiagnosticMessageFromJson, FormattedMessage, LocatedMessage; + show + Code, + DiagnosticMessageFromJson, + FormattedMessage, + LocatedMessage, + Message; /// Test that turning a message into json and back again retains the wanted /// information. main() { for (int i = 0; i < Severity.values.length; i++) { Severity severity = Severity.values[i]; + Code code = new Code("MyCodeName", null); + Message message = new Message(code); LocatedMessage locatedMessage1 = - new LocatedMessage(Uri.parse("what:ever/fun_1.dart"), 117, 2, null); + new LocatedMessage(Uri.parse("what:ever/fun_1.dart"), 117, 2, message); FormattedMessage formattedMessage2 = new FormattedMessage( null, "Formatted string #2", 13, 2, Severity.error, []); FormattedMessage formattedMessage3 = new FormattedMessage( @@ -30,6 +37,7 @@ main() { Uri.parse("what:ever/foo.dart"), Uri.parse("what:ever/bar.dart") ]); + expect(formattedMessage1.codeName, "MyCodeName"); DiagnosticMessageFromJson diagnosticMessageFromJson = new DiagnosticMessageFromJson.fromJson( @@ -72,6 +80,10 @@ void compareMessages(DiagnosticMessage a, DiagnosticMessage b) { expect(uriList1[i], uriList2[i]); } } + + String string1 = a.codeName; + String string2 = b.codeName; + expect(string1, string2); } void expect(Object actual, Object expect) { diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/can_get_rid_of_nnbd_issue_error.yaml.world.3.expect b/pkg/front_end/testcases/incremental_initialize_from_dill/can_get_rid_of_nnbd_issue_error.yaml.world.3.expect index a5bd64f6059..caa2322ce9c 100644 --- a/pkg/front_end/testcases/incremental_initialize_from_dill/can_get_rid_of_nnbd_issue_error.yaml.world.3.expect +++ b/pkg/front_end/testcases/incremental_initialize_from_dill/can_get_rid_of_nnbd_issue_error.yaml.world.3.expect @@ -5,7 +5,6 @@ main = main::main; // Error: This project cannot run with sound null safety, because one or more project dependencies do not // support null safety: // -// - package:foo // - package:bar // // Run 'pub outdated --mode=null-safety' to determine if versions of your diff --git a/pkg/kernel/problems.md b/pkg/kernel/problems.md index bc56e4c4417..073789e3d54 100644 --- a/pkg/kernel/problems.md +++ b/pkg/kernel/problems.md @@ -23,6 +23,8 @@ Normally this is not null (but it can be). `involvedFiles`: A possibly null list of uris involved in this message. Normally this is null. +`codeName`: A string identifing the specific error message. + These values are subject to change, but this file will be updated along with any such changes. On the code-side these are defined in `package:front_end/src/fasta/fasta_codes.dart`.