Issue 2694. Include pubspec.yaml content into the resolution salt.

Bug: https://github.com/dart-lang/linter/issues/2694
Change-Id: I239863703c70c6bd810be6847606e5fc6695073a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202265
Reviewed-by: Samuel Rawlins <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>

Author: scheglov

pkg/analyzer/lib/src/dart/analysis/driver.dart

@@ -43,6 +43,7 @@ import 'package:analyzer/src/summary/idl.dart';
 import 'package:analyzer/src/summary/package_bundle_reader.dart';
 import 'package:analyzer/src/summary2/ast_binary_flags.dart';
 import 'package:analyzer/src/util/file_paths.dart' as file_paths;
+import 'package:analyzer/src/workspace/pub.dart';
 import 'package:meta/meta.dart';
 
 /// This class computes [AnalysisResult]s for Dart files.
@@ -141,6 +142,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
 
   /// The salt to mix into all hashes used as keys for linked data.
   final Uint32List _saltForResolution = Uint32List(3 +
+      AnalysisOptionsImpl.signatureLength +
       AnalysisOptionsImpl.signatureLength +
       _declaredVariablesSignatureLength);
 
@@ -1899,6 +1901,25 @@ class AnalysisDriver implements AnalysisDriverGeneric {
     _saltForResolution[index] = enableDebugResolutionMarkers ? 1 : 0;
     index++;
 
+    // TODO(scheglov) Just combine everything into one signature.
+    {
+      var buffer = ApiSignature();
+
+      var workspace = analysisContext?.contextRoot.workspace;
+      // TODO(scheglov) Generalize?
+      if (workspace is PubWorkspace) {
+        buffer.addString(workspace.pubspecContent ?? '');
+      }
+
+      var bytes = buffer.toByteList();
+      _saltForResolution.setAll(
+        index,
+        // TODO(scheglov) Add a special method to ApiSignature?
+        Uint8List.fromList(bytes).buffer.asUint32List(),
+      );
+      index += AnalysisOptionsImpl.signatureLength;
+    }
+
     _saltForResolution.setAll(index, _analysisOptions.signature);
     index += AnalysisOptionsImpl.signatureLength;
 

pkg/analyzer/lib/src/workspace/package_build.dart

@@ -190,6 +190,13 @@ class PackageBuildWorkspace extends Workspace implements PubWorkspace {
   UriResolver get packageUriResolver => PackageBuildPackageUriResolver(
       this, PackageMapUriResolver(provider, packageMap));
 
+  @override
+  String? get pubspecContent {
+    try {
+      return _pubspecFile.readAsStringSync();
+    } catch (_) {}
+  }
+
   /// For some package file, which may or may not be a package source (it could
   /// be in `bin/`, `web/`, etc), find where its built counterpart will exist if
   /// its a generated source.

pkg/analyzer/lib/src/workspace/pub.dart

@@ -31,6 +31,13 @@ class PubWorkspace extends SimpleWorkspace {
     _theOnlyPackage = PubWorkspacePackage(root, this);
   }
 
+  /// Return the content of the pubspec file, `null` if cannot be read.
+  String? get pubspecContent {
+    try {
+      return _pubspecFile.readAsStringSync();
+    } catch (_) {}
+  }
+
   @override
   WorkspacePackage? findPackageFor(String filePath) {
     final Folder folder = provider.getFolder(filePath);

pkg/analyzer/test/src/dart/analysis/driver_caching_test.dart

@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/error/error.dart';
 import 'package:analyzer/src/dart/error/lint_codes.dart';
 import 'package:test/test.dart';
@@ -144,6 +145,66 @@ void f() {
     _assertNoLinkedCycles();
   }
 
+  test_lint_dependOnReferencedPackage_update_pubspec_addDependency() async {
+    useEmptyByteStore();
+
+    var aaaPackageRootPath = '$packagesRootPath/aaa';
+    newFile('$aaaPackageRootPath/lib/a.dart', content: '');
+
+    writeTestPackageConfig(
+      PackageConfigFileBuilder()
+        ..add(name: 'aaa', rootPath: aaaPackageRootPath),
+    );
+
+    // Configure with the lint.
+    writeTestPackageAnalysisOptionsFile(
+      AnalysisOptionsFileConfig(lints: ['depend_on_referenced_packages']),
+    );
+
+    // Configure without dependencies, but with a (required) name.
+    // So, the lint rule will be activated.
+    writeTestPackagePubspecYamlFile(
+      PubspecYamlFileConfig(
+        name: 'my_test',
+        dependencies: [],
+      ),
+    );
+
+    newFile(testFilePath, content: r'''
+// ignore:unused_import
+import 'package:aaa/a.dart';
+''');
+
+    // We don't have a dependency on `package:aaa`, so there is a lint.
+    _assertHasLintReported(
+      await _computeTestFileErrors(),
+      'depend_on_referenced_packages',
+    );
+
+    // The summary for the library was linked.
+    _assertContainsLinkedCycle({testFilePath}, andClear: true);
+
+    // We will recreate it with new pubspec.yaml content.
+    // But we will reuse the byte store, so can reuse summaries.
+    disposeAnalysisContextCollection();
+
+    // Add dependency on `package:aaa`.
+    writeTestPackagePubspecYamlFile(
+      PubspecYamlFileConfig(
+        name: 'my_test',
+        dependencies: [
+          PubspecYamlFileDependency(name: 'aaa'),
+        ],
+      ),
+    );
+
+    // With dependency on `package:aaa` added, no lint is reported.
+    expect(await _computeTestFileErrors(), isEmpty);
+
+    // Lints don't affect summaries, nothing should be linked.
+    _assertNoLinkedCycles();
+  }
+
   test_lints() async {
     useEmptyByteStore();
 
@@ -204,4 +265,17 @@ void f() {
   void _assertNoLinkedCycles() {
     expect(_linkedCycles, isEmpty);
   }
+
+  /// Note that we intentionally use this method, we don't want to use
+  /// [resolveFile] instead. Resolving a file will force to produce its
+  /// resolved AST, and as a result to recompute the errors.
+  ///
+  /// But this method is used to check returning errors from the cache, or
+  /// recomputing when the cache key is expected to be different.
+  Future<List<AnalysisError>> _computeTestFileErrors() async {
+    var errorsResult = await contextFor(testFilePath)
+        .currentSession
+        .getErrors2(testFilePath) as ErrorsResult;
+    return errorsResult.errors;
+  }
 }

pkg/analyzer/test/src/dart/resolution/context_collection_resolution.dart

@@ -347,22 +347,49 @@ class PubPackageResolutionTest extends ContextResolutionTest {
 }
 
 class PubspecYamlFileConfig {
+  final String? name;
   final String? sdkVersion;
+  final List<PubspecYamlFileDependency> dependencies;
 
-  PubspecYamlFileConfig({this.sdkVersion});
+  PubspecYamlFileConfig({
+    this.name,
+    this.sdkVersion,
+    this.dependencies = const [],
+  });
 
   String toContent() {
     var buffer = StringBuffer();
 
+    if (name != null) {
+      buffer.writeln('name: $name');
+    }
+
     if (sdkVersion != null) {
       buffer.writeln('environment:');
       buffer.writeln("  sdk: '$sdkVersion'");
     }
 
+    if (dependencies.isNotEmpty) {
+      buffer.writeln('dependencies:');
+      for (var dependency in dependencies) {
+        buffer.writeln('  ${dependency.name}: ${dependency.version}');
+      }
+    }
+
     return buffer.toString();
   }
 }
 
+class PubspecYamlFileDependency {
+  final String name;
+  final String version;
+
+  PubspecYamlFileDependency({
+    required this.name,
+    this.version = 'any',
+  });
+}
+
 mixin WithNoImplicitCastsMixin on PubPackageResolutionTest {
   Future<void> assertErrorsWithNoImplicitCasts(
     String code,