[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 <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This commit is contained in:
Jens Johansen 2020-11-05 09:43:23 +00:00 committed by commit-bot@chromium.org
parent 60a5c58196
commit 7b75bd8ece
7 changed files with 122 additions and 63 deletions

View file

@ -183,6 +183,8 @@ class FormattedMessage implements DiagnosticMessage {
Code<dynamic> 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<Uri> 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<String, Object> decoded = json.decode(jsonString);
@ -258,9 +263,10 @@ class DiagnosticMessageFromJson implements DiagnosticMessage {
: new List<String>.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<String, Object> toJson() {
@ -271,6 +277,7 @@ class DiagnosticMessageFromJson implements DiagnosticMessage {
"severity": severity.index,
"uri": uri?.toString(),
"involvedFiles": involvedFiles?.map((u) => u.toString())?.toList(),
"codeName": codeName,
};
}

View file

@ -36,6 +36,8 @@ abstract class DiagnosticMessage {
Severity get severity;
Iterable<Uri> get involvedFiles;
String get codeName;
}
/// This method is subject to change.

View file

@ -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<DiagnosticMessageFromJson> messages
in remainingComponentProblems.values) {
Set<Uri> strongModeNNBDPackageOptOutUris;
for (MapEntry<Uri, List<DiagnosticMessageFromJson>> entry
in remainingComponentProblems.entries) {
List<DiagnosticMessageFromJson> 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<LibraryBuilder> 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);

View file

@ -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<String, List<LibraryBuilder>> libraryByPackage = {};
Map<Package, Version> 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<Uri> involvedFiles = [];
List<String> dependencies = [];
libraryByPackage.forEach((String name, List<LibraryBuilder> 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<LibraryBuilder, Message> entry
@ -420,6 +368,68 @@ class SourceLoader extends Loader {
}
}
FormattedMessage giveCombinedErrorForNonStrongLibraries(
Set<LibraryBuilder> libraries,
{bool emitNonPackageErrors}) {
Map<String, List<LibraryBuilder>> libraryByPackage = {};
Map<Package, Version> 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<Uri> involvedFiles = [];
List<String> dependencies = [];
libraryByPackage.forEach((String name, List<LibraryBuilder> 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<int> getSource(List<int> bytes) {
// bytes is 0-terminated. We don't want that included.
if (bytes is Uint8List) {

View file

@ -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) {

View file

@ -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

View file

@ -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`.