[CFE] Don't issue bogus NNBD Strong error as package sets a language version

Before this CL, if one compiled a script as strong nnbd (with nnbd
enabled), where the script was marked as as a version where nnbd is enabled,
but where the script lived in a package with a version that was too low
one would get an error.
The reason is that the language version is set twice:
First when the library is created (but the file isn't read yet). This sets
the version from the package. Here an error was issued.
The second time is when the file is read and the file-version-marker is
parsed (e.g. @Dart = 2.9).

The error should only be given if - once the final verison has been set
(i.e. after parsing the file) - the version is "wrong".

To make matters worse the (bogus) error given even points to the files
first position which can make it seem like it specificly points to the
@Dart annotation and says it's wrong.

This CL fixes the error so it's only issued when it should be.
It does not fix pointing semi-weirdly.

Fixes #42323

Bug: 42323
Change-Id: Id84ca06b2e4eca5912e65798ed2d86f8c89d71a4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153769
Commit-Queue: Jens Johansen <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

Author: jensjoha

pkg/front_end/lib/src/fasta/source/source_library_builder.dart

@@ -453,15 +453,38 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
 
   LanguageVersion get languageVersion => _languageVersion;
 
+  void markLanguageVersionFinal() {
+    if (enableNonNullableInLibrary &&
+        (loader.nnbdMode == NnbdMode.Strong ||
+            loader.nnbdMode == NnbdMode.Agnostic)) {
+      // In strong and agnostic mode, the language version is not allowed to
+      // opt a library out of nnbd.
+      if (!isNonNullableByDefault) {
+        addPostponedProblem(messageStrongModeNNBDButOptOut,
+            _languageVersion.charOffset, _languageVersion.charCount, fileUri);
+        _languageVersion = new InvalidLanguageVersion(
+            fileUri,
+            _languageVersion.charOffset,
+            _languageVersion.charCount,
+            _languageVersion.isExplicit,
+            loader.target.currentSdkVersion);
+      }
+    }
+    _languageVersion.isFinal = true;
+  }
+
   @override
   void setLanguageVersion(Version version,
       {int offset: 0, int length: noLength, bool explicit: false}) {
+    assert(!_languageVersion.isFinal);
     if (languageVersion.isExplicit) return;
 
     if (version == null) {
       addPostponedProblem(
           messageLanguageVersionInvalidInDotPackages, offset, length, fileUri);
       if (_languageVersion is ImplicitLanguageVersion) {
+        // If the package set an OK version, but the file set an invalid version
+        // we want to use the package version.
         _languageVersion = new InvalidLanguageVersion(
             fileUri, offset, length, explicit, loader.target.currentSdkVersion);
         library.setLanguageVersion(_languageVersion.version);
@@ -480,6 +503,8 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
           length,
           fileUri);
       if (_languageVersion is ImplicitLanguageVersion) {
+        // If the package set an OK version, but the file set an invalid version
+        // we want to use the package version.
         _languageVersion = new InvalidLanguageVersion(
             fileUri, offset, length, explicit, loader.target.currentSdkVersion);
         library.setLanguageVersion(_languageVersion.version);
@@ -490,19 +515,6 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
     _languageVersion =
         new LanguageVersion(version, fileUri, offset, length, explicit);
     library.setLanguageVersion(version);
-
-    if (enableNonNullableInLibrary &&
-        (loader.nnbdMode == NnbdMode.Strong ||
-            loader.nnbdMode == NnbdMode.Agnostic)) {
-      // In strong and agnostic mode, the language version is not allowed to
-      // opt a library out of nnbd.
-      if (!isNonNullableByDefault) {
-        addPostponedProblem(
-            messageStrongModeNNBDButOptOut, offset, length, fileUri);
-        _languageVersion = new InvalidLanguageVersion(
-            fileUri, offset, length, explicit, loader.target.currentSdkVersion);
-      }
-    }
   }
 
   ConstructorReferenceBuilder addConstructorReference(Object name,
@@ -3780,6 +3792,7 @@ class LanguageVersion {
   final int charOffset;
   final int charCount;
   final bool isExplicit;
+  bool isFinal = false;
 
   LanguageVersion(this.version, this.fileUri, this.charOffset, this.charCount,
       this.isExplicit);
@@ -3807,6 +3820,7 @@ class InvalidLanguageVersion implements LanguageVersion {
   final int charCount;
   final bool isExplicit;
   final Version version;
+  bool isFinal = false;
 
   InvalidLanguageVersion(this.fileUri, this.charOffset, this.charCount,
       this.isExplicit, this.version);
@@ -3830,6 +3844,7 @@ class InvalidLanguageVersion implements LanguageVersion {
 class ImplicitLanguageVersion implements LanguageVersion {
   @override
   final Version version;
+  bool isFinal = false;
 
   ImplicitLanguageVersion(this.version);
 

pkg/front_end/lib/src/fasta/source/source_loader.dart

@@ -262,8 +262,10 @@ class SourceLoader extends Loader {
             enableNonNullable: library.isNonNullableByDefault),
         languageVersionChanged:
             (Scanner scanner, LanguageVersionToken version) {
-      library.setLanguageVersion(new Version(version.major, version.minor),
-          offset: version.offset, length: version.length, explicit: true);
+      if (!suppressLexicalErrors) {
+        library.setLanguageVersion(new Version(version.major, version.minor),
+            offset: version.offset, length: version.length, explicit: true);
+      }
       scanner.configuration = new ScannerConfiguration(
           enableTripleShift: library.enableTripleShiftInLibrary,
           enableExtensionMethods: library.enableExtensionMethodsInLibrary,
@@ -290,6 +292,7 @@ class SourceLoader extends Loader {
           importUri, library.fileUri, result.lineStarts, source);
     }
     library.issuePostponedProblems();
+    library.markLanguageVersionFinal();
     while (token is ErrorToken) {
       if (!suppressLexicalErrors) {
         ErrorToken error = token;
@@ -354,7 +357,7 @@ class SourceLoader extends Loader {
           // Part was included in multiple libraries. Skip it here.
           continue;
         }
-        Token tokens = await tokenize(part);
+        Token tokens = await tokenize(part, suppressLexicalErrors: true);
         if (tokens != null) {
           listener.uri = part.fileUri;
           parser.parseUnit(tokens);

pkg/front_end/test/incremental_load_from_dill_suite.dart

@@ -26,6 +26,8 @@ import 'package:front_end/src/api_prototype/experimental_flags.dart';
 import "package:front_end/src/api_prototype/memory_file_system.dart"
     show MemoryFileSystem;
 
+import 'package:front_end/src/base/nnbd_mode.dart' show NnbdMode;
+
 import 'package:front_end/src/base/processed_options.dart'
     show ProcessedOptions;
 
@@ -112,6 +114,7 @@ class Context extends ChainContext {
   @override
   Future<void> cleanUp(TestDescription description, Result result) async {
     await cleanupHelper?.outDir?.delete(recursive: true);
+    cleanupHelper?.outDir = null;
   }
 
   TestData cleanupHelper;
@@ -328,6 +331,12 @@ class NewWorldTest {
   Component component2;
   Component component3;
 
+  String doStringReplacements(String input) {
+    String output = input.replaceAll("%NNBD_VERSION_MARKER%",
+        "${enableNonNullableVersion.major}.${enableNonNullableVersion.minor}");
+    return output;
+  }
+
   Future<Null> newWorldTest(
       TestData data,
       Context context,
@@ -458,6 +467,9 @@ class NewWorldTest {
         if (filename == ".packages") {
           packagesUri = uri;
         }
+        if (world["enableStringReplacement"] == true) {
+          data = doStringReplacements(data);
+        }
         fs.entityForUri(uri).writeAsStringSync(data);
       }
       if (world["dotPackagesFile"] != null) {
@@ -481,6 +493,16 @@ class NewWorldTest {
                       throw "Error on parsing experiments flags: $e");
           options.experimentalFlags = experimentalFlags;
         }
+        if (world["nnbdMode"] != null) {
+          String nnbdMode = world["nnbdMode"];
+          switch (nnbdMode) {
+            case "strong":
+              options.nnbdMode = NnbdMode.Strong;
+              break;
+            default:
+              throw "Not supported nnbd mode: $nnbdMode";
+          }
+        }
       }
       if (packagesUri != null) {
         options.packagesFileUri = packagesUri;

pkg/front_end/testcases/incremental_initialize_from_dill/issue_42323.yaml

@@ -0,0 +1,33 @@
+# Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+# for details. All rights reserved. Use of this source code is governed by a
+# BSD-style license that can be found in the LICENSE.md file.
+
+# This is not actually an incremental compiler test as much as a compiler test.
+# Test what happens when running a package file as strong NNBD (and it's marked
+# with a matching language version) in a package that's with a lower language
+# version (nothing should really happen --- it should just work).
+# See https://github.com/dart-lang/sdk/issues/42323.
+
+type: newworld
+worlds:
+  - entry: bin/runMe.dart
+    experiments: non-nullable
+    nnbdMode: strong
+    enableStringReplacement: true
+    sources:
+      .dart_tool/package_config.json: |
+        {
+          "configVersion": 2,
+          "packages": [
+            {
+              "name": "issue42323",
+              "rootUri": "../",
+              "packageUri": "lib/",
+              "languageVersion": "2.8"
+            }
+          ]
+        }
+      bin/runMe.dart: |
+        // @dart = %NNBD_VERSION_MARKER%
+        main() {}
+    expectedLibraryCount: 1

pkg/front_end/testcases/incremental_initialize_from_dill/issue_42323.yaml.world.1.expect

@@ -0,0 +1,5 @@
+main = <No Member>;
+library from "org-dartlang-test:///bin/runMe.dart" as run {
+
+  static method main() → dynamic {}
+}

pkg/front_end/testcases/incremental_initialize_from_dill/issue_42323_prime.yaml

@@ -0,0 +1,33 @@
+# Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+# for details. All rights reserved. Use of this source code is governed by a
+# BSD-style license that can be found in the LICENSE.md file.
+
+# This is not actually an incremental compiler test as much as a compiler test.
+# Test what happens when running a package file as strong NNBD (and it's marked
+# with a non-nnbd language version) in a package that's with a lower language
+# version (an error should be reported).
+# Good "variant" of https://github.com/dart-lang/sdk/issues/42323.
+
+type: newworld
+worlds:
+  - entry: bin/runMe.dart
+    experiments: non-nullable
+    nnbdMode: strong
+    errors: true
+    sources:
+      .dart_tool/package_config.json: |
+        {
+          "configVersion": 2,
+          "packages": [
+            {
+              "name": "issue42323",
+              "rootUri": "../",
+              "packageUri": "lib/",
+              "languageVersion": "2.8"
+            }
+          ]
+        }
+      bin/runMe.dart: |
+        // @dart = 2.8
+        main() {}
+    expectedLibraryCount: 1

pkg/front_end/testcases/incremental_initialize_from_dill/issue_42323_prime.yaml.world.1.expect

@@ -0,0 +1,12 @@
+main = <No Member>;
+library from "org-dartlang-test:///bin/runMe.dart" as run {
+//
+// Problems in library:
+//
+// org-dartlang-test:///bin/runMe.dart:1:1: Error: A library can't opt out of null safety by default, when using sound null safety.
+// // @dart = 2.8
+// ^^^^^^^^^^^^^^
+//
+
+  static method main() → dynamic {}
+}

tests/dart2js/inferrer_is_int_test.dart

@@ -6,7 +6,7 @@
 // literal might become an int at runtime.
 
 import "package:expect/expect.dart";
-import '../language_2/compiler_annotations.dart';
+import '../language/compiler_annotations.dart';
 
 @DontInline()
 callWithStringAndDouble(value) {