Skip to content

Commit af9cc50

Browse files
srawlinsCommit Queue
authored andcommitted
DAS plugins: Normalize a plugin path
Change-Id: I323a5e4f5c7660ebf0f7c1bbee8b62a32817ba71 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/396681 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
1 parent 6526f6c commit af9cc50

File tree

4 files changed

+149
-9
lines changed

4 files changed

+149
-9
lines changed

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

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,8 @@ final class AnalysisOptionsBuilder {
195195
}
196196
}
197197

198-
void _applyPluginsOptions(YamlNode? plugins) {
198+
void _applyPluginsOptions(
199+
YamlNode? plugins, ResourceProvider? resourceProvider) {
199200
if (plugins is! YamlMap) {
200201
return;
201202
}
@@ -232,8 +233,23 @@ final class AnalysisOptionsBuilder {
232233
source = VersionedPluginSource(constraint: value);
233234
} else {
234235
var pathSource = pluginNode.valueAt(AnalysisOptionsFile.path);
235-
if (pathSource case YamlScalar(:String value)) {
236-
source = PathPluginSource(path: value);
236+
if (pathSource case YamlScalar(value: String pathValue)) {
237+
var file = this.file;
238+
assert(
239+
file != null,
240+
"AnalysisOptionsImpl must be initialized with a non-null 'file' if "
241+
'plugins are specified with path constraints.',
242+
);
243+
if (file != null &&
244+
resourceProvider != null &&
245+
resourceProvider.pathContext.isRelative(pathValue)) {
246+
// We need to store the absolute path, before this value is used in
247+
// a synthetic pub package.
248+
pathValue =
249+
resourceProvider.pathContext.join(file.parent.path, pathValue);
250+
pathValue = resourceProvider.pathContext.normalize(pathValue);
251+
}
252+
source = PathPluginSource(path: pathValue);
237253
}
238254
}
239255

@@ -379,8 +395,11 @@ class AnalysisOptionsImpl implements AnalysisOptions {
379395
/// [optionsMap].
380396
///
381397
/// Optionally pass [file] as the file where the YAML can be found.
382-
factory AnalysisOptionsImpl.fromYaml(
383-
{required YamlMap optionsMap, File? file}) {
398+
factory AnalysisOptionsImpl.fromYaml({
399+
required YamlMap optionsMap,
400+
File? file,
401+
ResourceProvider? resourceProvider,
402+
}) {
384403
var builder = AnalysisOptionsBuilder()..file = file;
385404

386405
var analyzer = optionsMap.valueAt(AnalysisOptionsFile.analyzer);
@@ -433,7 +452,7 @@ class AnalysisOptionsImpl implements AnalysisOptions {
433452

434453
// Process the 'plugins' option.
435454
var plugins = optionsMap.valueAt(AnalysisOptionsFile.plugins);
436-
builder._applyPluginsOptions(plugins);
455+
builder._applyPluginsOptions(plugins, resourceProvider);
437456

438457
var ruleConfigs = parseLinterSection(optionsMap);
439458
if (ruleConfigs != null) {

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,10 @@ class ContextBuilderImpl {
202202
for (var entry in optionsMappings) {
203203
var file = entry.value;
204204
var options = AnalysisOptionsImpl.fromYaml(
205-
file: file, optionsMap: provider.getOptionsFromFile(file));
205+
optionsMap: provider.getOptionsFromFile(file),
206+
file: file,
207+
resourceProvider: resourceProvider,
208+
);
206209

207210
_optionsMap.add(entry.key, options);
208211
}
@@ -291,6 +294,7 @@ class ContextBuilderImpl {
291294
options = AnalysisOptionsImpl.fromYaml(
292295
optionsMap: provider.getOptionsFromFile(optionsFile),
293296
file: optionsFile,
297+
resourceProvider: resourceProvider,
294298
);
295299
} catch (e) {
296300
// Ignore exception.

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,8 +553,9 @@ class ContextLocatorImpl {
553553
AnalysisOptionsProvider(workspace.createSourceFactory(null, null));
554554

555555
var options = AnalysisOptionsImpl.fromYaml(
556-
file: optionsFile,
557556
optionsMap: provider.getOptionsFromFile(optionsFile),
557+
file: optionsFile,
558+
resourceProvider: resourceProvider,
558559
);
559560

560561
return options.enabledLegacyPluginNames.toSet();

pkg/analyzer/test/src/options/analysis_options_test.dart

Lines changed: 117 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'package:analyzer/dart/analysis/analysis_options.dart';
56
import 'package:analyzer/error/error.dart';
7+
import 'package:analyzer/file_system/file_system.dart';
8+
import 'package:analyzer/file_system/memory_file_system.dart';
69
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
710
import 'package:analyzer/src/dart/analysis/analysis_options.dart';
811
import 'package:analyzer/src/error/codes.dart';
@@ -21,11 +24,16 @@ main() {
2124
class AnalysisOptionsTest {
2225
final AnalysisOptionsProvider optionsProvider = AnalysisOptionsProvider();
2326

27+
final ResourceProvider resourceProvider = MemoryResourceProvider();
28+
2429
// TODO(srawlins): Add tests that exercise
2530
// `optionsProvider.getOptionsFromString` throwing an exception.
2631
AnalysisOptionsImpl parseOptions(String content) =>
2732
AnalysisOptionsImpl.fromYaml(
28-
optionsMap: optionsProvider.getOptionsFromString(content));
33+
optionsMap: optionsProvider.getOptionsFromString(content),
34+
file: resourceProvider.getFile('/project/analysis_options.yaml'),
35+
resourceProvider: resourceProvider,
36+
);
2937

3038
test_analyzer_cannotIgnore() {
3139
var analysisOptions = parseOptions('''
@@ -283,6 +291,114 @@ analyzer:
283291
expect(analysisOptions.propagateLinterExceptions, true);
284292
}
285293

294+
test_analyzer_plugins_pathConstraint() {
295+
var analysisOptions = parseOptions('''
296+
plugins:
297+
plugin_one:
298+
path: /foo/bar
299+
''');
300+
301+
var configuration = analysisOptions.pluginConfigurations.single;
302+
expect(configuration.isEnabled, isTrue);
303+
expect(configuration.name, 'plugin_one');
304+
expect(
305+
configuration.source,
306+
isA<PathPluginSource>().having(
307+
(e) => e.toYaml(name: 'plugin_one'),
308+
'toYaml',
309+
'''
310+
plugin_one:
311+
path: /foo/bar
312+
''',
313+
),
314+
);
315+
}
316+
317+
test_analyzer_plugins_pathConstraint_relative() {
318+
var analysisOptions = parseOptions('''
319+
plugins:
320+
plugin_one:
321+
path: foo/bar
322+
''');
323+
324+
var configuration = analysisOptions.pluginConfigurations.single;
325+
expect(configuration.isEnabled, isTrue);
326+
expect(configuration.name, 'plugin_one');
327+
expect(
328+
configuration.source,
329+
isA<PathPluginSource>().having(
330+
(e) => e.toYaml(name: 'plugin_one'),
331+
'toYaml',
332+
'''
333+
plugin_one:
334+
path: /project/foo/bar
335+
''',
336+
),
337+
);
338+
}
339+
340+
test_analyzer_plugins_pathConstraint_relativeNonNormal() {
341+
var analysisOptions = parseOptions('''
342+
plugins:
343+
plugin_one:
344+
path: .././foo/bar/../baz
345+
''');
346+
347+
var configuration = analysisOptions.pluginConfigurations.single;
348+
expect(configuration.isEnabled, isTrue);
349+
expect(configuration.name, 'plugin_one');
350+
expect(
351+
configuration.source,
352+
isA<PathPluginSource>().having(
353+
(e) => e.toYaml(name: 'plugin_one'),
354+
'toYaml',
355+
'''
356+
plugin_one:
357+
path: /foo/baz
358+
''',
359+
),
360+
);
361+
}
362+
363+
test_analyzer_plugins_scalarConstraint() {
364+
var analysisOptions = parseOptions('''
365+
plugins:
366+
plugin_one: ^1.2.3
367+
''');
368+
369+
var configuration = analysisOptions.pluginConfigurations.single;
370+
expect(configuration.isEnabled, isTrue);
371+
expect(configuration.name, 'plugin_one');
372+
expect(
373+
configuration.source,
374+
isA<VersionedPluginSource>().having(
375+
(e) => e.toYaml(name: 'plugin_one'),
376+
'toYaml',
377+
' plugin_one: ^1.2.3\n',
378+
),
379+
);
380+
}
381+
382+
test_analyzer_plugins_versionConstraint() {
383+
var analysisOptions = parseOptions('''
384+
plugins:
385+
plugin_one:
386+
version: ^1.2.3
387+
''');
388+
389+
var configuration = analysisOptions.pluginConfigurations.single;
390+
expect(configuration.isEnabled, isTrue);
391+
expect(configuration.name, 'plugin_one');
392+
expect(
393+
configuration.source,
394+
isA<VersionedPluginSource>().having(
395+
(e) => e.toYaml(name: 'plugin_one'),
396+
'toYaml',
397+
' plugin_one: ^1.2.3\n',
398+
),
399+
);
400+
}
401+
286402
test_codeStyle_format_false() {
287403
var analysisOptions = parseOptions('''
288404
code-style:

0 commit comments

Comments
 (0)