Fix PathFilter created when the analysis options file with excludes is not in the context root.

Change-Id: I242473d5fddbe5e3abaa994766959e207453ffb0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197114
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Devon Carew <[email protected]>

Author: scheglov

pkg/analyzer/lib/src/source/path_filter.dart

@@ -5,29 +5,35 @@
 import 'package:analyzer/src/util/glob.dart';
 import 'package:path/path.dart' as path;
 
-/// Filter paths against a set of [_ignorePatterns] relative to a [root]
-/// directory. Paths outside of [root] are also ignored.
+/// Filter paths against a set of [_ignorePatterns] relative to a
+/// [ignorePatternsRoot] directory. Paths outside of [includedRoot] are also
+/// ignored.
 class PathFilter {
   /// The path context to use when manipulating paths.
   final path.Context pathContext;
 
+  /// The path in which files are considered to be included.
+  final String includedRoot;
+
   /// Path that all ignore patterns are relative to.
-  final String root;
+  final String ignorePatternsRoot;
 
   /// List of ignore patterns that paths are tested against.
   final List<Glob> _ignorePatterns = <Glob>[];
 
-  /// Construct a new path filter rooted at [root] with [ignorePatterns].
+  /// Construct a new path filter rooted at [includedRoot],
+  /// with [ignorePatterns] that are relative to [ignorePatternsRoot].
   /// If [pathContext] is not specified, then the system path context is used.
-  PathFilter(this.root, List<String> ignorePatterns,
+  PathFilter(
+      this.includedRoot, this.ignorePatternsRoot, List<String> ignorePatterns,
       [path.Context? pathContext])
       : pathContext = pathContext ?? path.context {
     setIgnorePatterns(ignorePatterns);
   }
 
   /// Returns true if [path] should be ignored. A path is ignored if it is not
-  /// contained in [root] or matches one of the ignore patterns.
-  /// [path] is absolute or relative to [root].
+  /// contained in [includedRoot] or matches one of the ignore patterns.
+  /// [path] is absolute or relative to [includedRoot].
   bool ignored(String path) {
     path = _canonicalize(path);
     return !_contained(path) || _match(path);
@@ -53,24 +59,21 @@ class PathFilter {
     return sb.toString();
   }
 
-  /// Returns the absolute path of [path], relative to [root].
+  /// Returns the absolute path of [path], relative to [includedRoot].
   String _canonicalize(String path) =>
-      pathContext.normalize(pathContext.join(root, path));
+      pathContext.normalize(pathContext.join(includedRoot, path));
 
-  /// Returns true when [path] is contained inside [root].
-  bool _contained(String path) => path.startsWith(root);
+  /// Returns true when [path] is contained inside [includedRoot].
+  bool _contained(String path) => path.startsWith(includedRoot);
 
   /// Returns true if [path] matches any ignore patterns.
   bool _match(String path) {
-    path = _relative(path);
+    var relative = pathContext.relative(path, from: ignorePatternsRoot);
     for (Glob glob in _ignorePatterns) {
-      if (glob.matches(path)) {
+      if (glob.matches(relative)) {
         return true;
       }
     }
     return false;
   }
-
-  /// Returns the relative portion of [path] from [root].
-  String _relative(String path) => pathContext.relative(path, from: root);
 }

pkg/analyzer/test/source/path_filter_test.dart

@@ -8,53 +8,68 @@ import 'package:test/test.dart';
 
 main() {
   String root(String path) => context.absolute(context.normalize(path));
+
+  PathFilter withSingleRoot(String root, List<String> ignorePatterns) {
+    return PathFilter(root, root, ignorePatterns, context);
+  }
+
   group('PathFilterTest', () {
-    setUp(() {});
-    tearDown(() {});
     test('test_ignoreEverything', () {
-      var filter = PathFilter(root('/'), ['*'], context);
+      var filter = withSingleRoot(root('/'), ['*']);
       expect(filter.ignored('a'), isTrue);
     });
+
     test('test_ignoreFile', () {
-      var filter = PathFilter(root('/'), ['apple'], context);
+      var filter = withSingleRoot(root('/'), ['apple']);
       expect(filter.ignored('apple'), isTrue);
       expect(filter.ignored('banana'), isFalse);
     });
+
     test('test_ignoreMultipleFiles', () {
-      var filter = PathFilter(root('/'), ['apple', 'banana'], context);
+      var filter = withSingleRoot(root('/'), ['apple', 'banana']);
       expect(filter.ignored('apple'), isTrue);
       expect(filter.ignored('banana'), isTrue);
     });
+
     test('test_ignoreSubDir', () {
-      var filter = PathFilter(root('/'), ['apple/*'], context);
+      var filter = withSingleRoot(root('/'), ['apple/*']);
       expect(filter.ignored('apple/banana'), isTrue);
       expect(filter.ignored('apple/banana/cantaloupe'), isFalse);
     });
+
     test('test_ignoreTree', () {
-      var filter = PathFilter(root('/'), ['apple/**'], context);
+      var filter = withSingleRoot(root('/'), ['apple/**']);
       expect(filter.ignored('apple/banana'), isTrue);
       expect(filter.ignored('apple/banana/cantaloupe'), isTrue);
     });
+
     test('test_ignoreSdkExt', () {
-      var filter = PathFilter(root('/'), ['sdk_ext/**'], context);
+      var filter = withSingleRoot(root('/'), ['sdk_ext/**']);
       expect(filter.ignored('sdk_ext/entry.dart'), isTrue);
       expect(filter.ignored('sdk_ext/lib/src/part.dart'), isTrue);
     });
+
     test('test_outsideRoot', () {
-      var filter =
-          PathFilter(root('/workspace/dart/sdk'), ['sdk_ext/**'], context);
+      var filter = withSingleRoot(root('/workspace/dart/sdk'), ['sdk_ext/**']);
       expect(filter.ignored('/'), isTrue);
       expect(filter.ignored('/workspace'), isTrue);
       expect(filter.ignored('/workspace/dart'), isTrue);
       expect(filter.ignored('/workspace/dart/sdk'), isFalse);
       expect(filter.ignored('/workspace/dart/../dart/sdk'), isFalse);
     });
+
     test('test_relativePaths', () {
-      var filter =
-          PathFilter(root('/workspace/dart/sdk'), ['sdk_ext/**'], context);
+      var filter = withSingleRoot(root('/workspace/dart/sdk'), ['sdk_ext/**']);
       expect(filter.ignored('../apple'), isTrue);
       expect(filter.ignored('../sdk/main.dart'), isFalse);
       expect(filter.ignored('../sdk/sdk_ext/entry.dart'), isTrue);
     });
+
+    test('different ignore patterns root', () {
+      var filter = PathFilter(
+          root('/home/my'), root('/home'), ['my/test/ignored/*.dart'], context);
+      expect(filter.ignored(root('/home/my/lib/a.dart')), isFalse);
+      expect(filter.ignored(root('/home/my/test/ignored/b.dart')), isTrue);
+    });
   });
 }

pkg/analyzer_cli/lib/src/driver.dart

@@ -505,7 +505,16 @@ class _AnalysisContextProvider {
 
   /// TODO(scheglov) Use analyzedFiles()
   PathFilter get pathFilter {
-    return PathFilter(analysisContext.contextRoot.root.path,
+    var contextRoot = analysisContext.contextRoot;
+    var optionsFile = contextRoot.optionsFile;
+
+    // If there is no options file, there can be no excludes.
+    if (optionsFile == null) {
+      return PathFilter(contextRoot.root.path, contextRoot.root.path, []);
+    }
+
+    // Exclude patterns are relative to the directory with the options file.
+    return PathFilter(contextRoot.root.path, optionsFile.parent2.path,
         analysisContext.analysisOptions.excludePatterns);
   }
 

pkg/analyzer_cli/test/data/exclude_portion_of_inner_context/analysis_options.yaml

@@ -0,0 +1,3 @@
+analyzer:
+  exclude:
+    - inner/lib/excluded_error.dart

pkg/analyzer_cli/test/data/exclude_portion_of_inner_context/inner/.dart_tool/package_config.json

@@ -0,0 +1,11 @@
+{
+  "configVersion": 2,
+  "packages": [
+    {
+      "name": "inner",
+      "rootUri": "../",
+      "packageUri": "lib",
+      "languageVersion": "2.9"
+    }
+  ]
+}

pkg/analyzer_cli/test/data/exclude_portion_of_inner_context/inner/lib/excluded_error.dart

@@ -0,0 +1 @@
+ExcludedUndefinedClassInInner x = null;

pkg/analyzer_cli/test/data/exclude_portion_of_inner_context/inner/lib/not_excluded_error.dart

@@ -0,0 +1 @@
+IncludedUndefinedClassInInner f = null;

pkg/analyzer_cli/test/data/exclude_portion_of_inner_context/inner/pubspec.yaml

@@ -0,0 +1,2 @@
+# Note `.dart_tool/package_config.json` - it, not this file, makes a new analysis context.
+name: inner

pkg/analyzer_cli/test/data/exclude_portion_of_inner_context/lib/not_excluded_error.dart

@@ -0,0 +1,2 @@
+/// Should not be reported, we analyze only `inner`.
+IncludedUndefinedClassInOuter f = null;

pkg/analyzer_cli/test/driver_test.dart

@@ -396,6 +396,16 @@ class OptionsTest extends BaseTest {
     _expectUndefinedClassErrorsWithoutExclusions();
   }
 
+  Future<void> test_analysisOptions_excludes_inner() async {
+    await drive('data/exclude_portion_of_inner_context/inner',
+        options: 'data/exclude_portion_of_inner_context/$analysisOptionsYaml');
+    expect(
+      bulletToDash(outSink),
+      contains("error - Undefined class 'IncludedUndefinedClassInInner'"),
+    );
+    expect(outSink.toString(), contains('1 error found.'));
+  }
+
   Future<void>
       test_analysisOptions_excludesRelativeToAnalysisOptions_explicit() async {
     // The exclude is relative to the project, not/ the analyzed path, and it