Skip to content

Commit

Permalink
[stable] Fix resolving package includes in options file.
Browse files Browse the repository at this point in the history
Fixes #56047


Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/376500
Cherry-pick-request: #56457
Change-Id: I3cb5ee316e4fe5564c0e5234d08566fec84de759
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380205
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Kevin Chisholm <[email protected]>
Reviewed-by: Phil Quitslund <[email protected]>
  • Loading branch information
keertip authored and Commit Queue committed Aug 13, 2024
1 parent 061b0b2 commit 14ff0ca
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 22 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

### Tools

#### Analyzer

- Fix for resolving `include:` in `analysis_options.yaml` file in a nested
folder in the workspace. [#56047][]

[#56047]: https://github.com/dart-lang/sdk/issues/56047

#### Wasm compiler (dart2wasm)

- Fix source maps generated by `dart compile wasm` when optimizations are
Expand Down
17 changes: 4 additions & 13 deletions pkg/analysis_server/test/lsp/diagnostic_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -505,19 +505,12 @@ version: latest

/// Ensure lints included from another package work when there are multiple
/// workspace folders.
///
/// https://github.com/dart-lang/sdk/issues/56047
@skippedTest
Future<void> test_lints_includedFromPackage() async {
// FailingTest() doesn't handle timeouts so this is marked as skipped.
// Needs to be manually updated when
// https://github.com/dart-lang/sdk/issues/56047 is fixed.

var rootWorkspacePath = '$packagesRootPath/root';

// Set up a project with an analysis_options that enables a lint.
var lintsPackagePath = '$rootWorkspacePath/my_lints';
newFile('$lintsPackagePath/lib/pubspec.yaml', '''
newFile('$lintsPackagePath/pubspec.yaml', '''
name: my_lints
''');
newFile('$lintsPackagePath/lib/analysis_options.yaml', '''
Expand All @@ -529,11 +522,9 @@ linter:

// Set up a project that imports the analysis_options and violates the lint.
var projectPackagePath = '$rootWorkspacePath/my_project';
writePackageConfig(
projectPackagePath,
config: (PackageConfigFileBuilder()
..add(name: 'my_lints', rootPath: lintsPackagePath)),
);
writePackageConfig(projectPackagePath,
config: (PackageConfigFileBuilder()
..add(name: 'my_lints', rootPath: lintsPackagePath)));
newFile('$projectPackagePath/analysis_options.yaml', '''
include: package:my_lints/analysis_options.yaml
Expand Down
18 changes: 9 additions & 9 deletions pkg/analyzer/lib/src/dart/analysis/context_locator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -312,15 +312,6 @@ class ContextLocatorImpl implements ContextLocator {
}
var buildGnFile = folder.getExistingFile(file_paths.buildGn);

if (localOptionsFile != null) {
(containingRoot as ContextRootImpl).optionsFileMap[folder] =
localOptionsFile;
// Add excluded globs.
var excludes =
_getExcludedGlobs(localOptionsFile, containingRoot.workspace);
containingRoot.excludedGlobs.addAll(excludes);
}

//
// Create a context root for the given [folder] if a packages or build file
// is locally specified.
Expand Down Expand Up @@ -350,6 +341,15 @@ class ContextLocatorImpl implements ContextLocator {
excludedGlobs = _getExcludedGlobs(root.optionsFile, workspace);
root.excludedGlobs = excludedGlobs;
}

if (localOptionsFile != null) {
(containingRoot as ContextRootImpl).optionsFileMap[folder] =
localOptionsFile;
// Add excluded globs.
var excludes =
_getExcludedGlobs(localOptionsFile, containingRoot.workspace);
containingRoot.excludedGlobs.addAll(excludes);
}
_createContextRootsIn(roots, visited, folder, excludedFolders,
containingRoot, excludedGlobs, optionsFile, packagesFile);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,91 @@ workspaces
root: /home/test/nested
''');
}

test_packageConfigWorkspace_singleAnalysisOptions_with_include() {
var workspaceRootPath = '/home';

var fooPackagePath = '$workspaceRootPath/foo';
newFile('$fooPackagePath/pubspec.yaml', '''
name: foo
''');
newFile('$fooPackagePath/lib/included.yaml', r'''
linter:
rules:
- empty_statements
''');
var packageConfigFileBuilder = PackageConfigFileBuilder()
..add(name: 'foo', rootPath: fooPackagePath);
newPackageConfigJsonFile(
fooPackagePath,
packageConfigFileBuilder.toContent(toUriStr: toUriStr),
);

var testPackagePath = '$workspaceRootPath/test';
packageConfigFileBuilder.add(name: 'test', rootPath: testPackagePath);

newPackageConfigJsonFile(
testPackagePath,
packageConfigFileBuilder.toContent(toUriStr: toUriStr),
);
newFile('$testPackagePath/pubspec.yaml', '''
name: test
''');

var optionsFile = newAnalysisOptionsYamlFile(testPackagePath, r'''
include: package:foo/included.yaml
linter:
rules:
- unnecessary_parenthesis
''');
newFile('$testPackagePath/lib/a.dart', '');

var collection = _newCollection(includedPaths: [workspaceRootPath]);
var analysisContext = collection.contextFor(testPackagePath);
var analysisOptions =
analysisContext.getAnalysisOptionsForFile(optionsFile);

expect(
analysisOptions.lintRules.map((e) => e.name),
unorderedEquals(['empty_statements', 'unnecessary_parenthesis']),
);

_assertWorkspaceCollectionText(workspaceRootPath, r'''
contexts
/home/foo
packagesFile: /home/foo/.dart_tool/package_config.json
workspace: workspace_0
analyzedFiles
/home/test
packagesFile: /home/test/.dart_tool/package_config.json
workspace: workspace_1
analyzedFiles
/home/test/lib/a.dart
uri: package:test/a.dart
analysisOptions_0
workspacePackage_1_0
analysisOptions
analysisOptions_0: /home/test/analysis_options.yaml
workspaces
workspace_0: PackageConfigWorkspace
root: /home/foo
workspace_1: PackageConfigWorkspace
root: /home/test
pubPackages
workspacePackage_1_0: PubPackage
root: /home/test
''');
}

AnalysisContextCollectionImpl _newCollection(
{required List<String> includedPaths}) {
return AnalysisContextCollectionImpl(
resourceProvider: resourceProvider,
includedPaths: includedPaths,
sdkPath: sdkRoot.path,
);
}
}

mixin AnalysisContextCollectionTestMixin on ResourceProviderMixin {
Expand Down

0 comments on commit 14ff0ca

Please sign in to comment.