DAS plugins: warn when plugins found in inner options file

Work towards #61481

Change-Id: I16d5333de6b5b195f629ec51ee582d793431f139
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/450220
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: srawlins

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -115,6 +115,10 @@ AnalysisOptionsWarningCode.MULTIPLE_PLUGINS:
   notes: |-
     The fix is to remove the plugin name. Might not be worth the cost if we
     can address the question of if/how we're supporting plugins.
+AnalysisOptionsWarningCode.PLUGINS_IN_INNER_OPTIONS:
+  status: needsFix
+  notes: |-
+    The fix is to delete the 'plugins' section.
 AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE:
   status: noFix
   notes: |-

pkg/analysis_server_plugin/doc/using_plugins.md

@@ -1,15 +1,23 @@
 # Using plugins
 
-This document describes how to enable an analyzer plugin. An analyzer plugin
-can be enabled for a given package, so that the plugin can report diagnostics
-(lints and warnings) and offer quick fixes. Plugins are enabled via the
-`analysis_options.yaml` file under the top-level `plugins` section:
+This document describes how to enable analyzer plugins. Analyzer plugins can
+report diagnostics (lints and warnings) in an IDE and also at the command line
+(with `dart analyze` or `flutter analyze`). Analyzer plugins can also offer
+quick fixes and assists in an IDE. A set of analyzer plugins can be enabled for
+a given package or [workspace][] via the `analysis_options.yaml` file, at the
+root of the package or workspace source tree. Analyzer plugins cannot be
+enabled, disabled, or otherwise specified or configured in a nested analysis
+options file
+
+Analyzer plugins are specified in the top-level `plugins` section:
 
 ```yaml
 plugins:
   my_plugin: ^1.0.0
 ```
 
+[workspace]: https://dart.dev/tools/pub/workspaces
+
 Note: This is similar to how analyzer plugins are enabled in the [legacy][]
 analyzer plugin system. However, in the legacy system, this `plugins` section
 is listed under the top-level `analyzer` section. In the new analyzer plugin
@@ -72,4 +80,4 @@ use a comment like the following:
 // ignore: some_plugin/some_code
 
 // ignore_for_file: some_plugin/some_code
-```
+```

pkg/analyzer/lib/src/analysis_options/error/option_codes.g.dart

@@ -375,6 +375,24 @@ class AnalysisOptionsWarningCode extends DiagnosticCodeWithExpectedTypes {
     expectedTypes: [ExpectedType.string],
   );
 
+  /// An error code indicating plugins have been specified in an "inner"
+  /// analysis options file.
+  ///
+  /// Parameters:
+  /// String contextRoot: the root of the analysis context
+  static const AnalysisOptionsWarningTemplate<
+    LocatableDiagnostic Function({required String contextRoot})
+  >
+  pluginsInInnerOptions = AnalysisOptionsWarningTemplate(
+    'PLUGINS_IN_INNER_OPTIONS',
+    "Plugins can only be specified in the root of a pub workspace or the root "
+        "of a package that isn't in a workspace.",
+    correctionMessage:
+        "Try specifying plugins in an analysis options file at '{0}'.",
+    withArguments: _withArgumentsPluginsInInnerOptions,
+    expectedTypes: [ExpectedType.string],
+  );
+
   /// An error code indicating a specified include file includes itself recursively.
   ///
   /// Parameters:
@@ -671,6 +689,12 @@ class AnalysisOptionsWarningCode extends DiagnosticCodeWithExpectedTypes {
     return LocatableDiagnosticImpl(multiplePlugins, [p0]);
   }
 
+  static LocatableDiagnostic _withArgumentsPluginsInInnerOptions({
+    required String contextRoot,
+  }) {
+    return LocatableDiagnosticImpl(pluginsInInnerOptions, [contextRoot]);
+  }
+
   static LocatableDiagnostic _withArgumentsRecursiveIncludeFile({
     required Object p0,
     required Object p1,

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

@@ -77,11 +77,12 @@ List<Diagnostic> analyzeAnalysisOptions(
   }
 
   // Validates the specified options and any included option files.
-  void validate(Source source, YamlMap options) {
+  void validate(Source source, YamlMap options, {required String contextRoot}) {
     var isSourcePrimary = initialIncludeSpan == null;
     var validationErrors = OptionsFileValidator(
       source,
       sdkVersionConstraint: sdkVersionConstraint,
+      contextRoot: contextRoot,
       isPrimarySource: isSourcePrimary,
       optionsProvider: optionsProvider,
       sourceFactory: sourceFactory,
@@ -168,7 +169,7 @@ List<Diagnostic> analyzeAnalysisOptions(
         var includedOptions = optionsProvider.getOptionsFromString(
           includedSource.contents.data,
         );
-        validate(includedSource, includedOptions);
+        validate(includedSource, includedOptions, contextRoot: contextRoot);
         firstPluginName ??= _firstPluginName(includedOptions);
         // Validate the 'plugins' option in [options], taking into account any
         // plugins enabled by [includedOptions].
@@ -222,7 +223,7 @@ List<Diagnostic> analyzeAnalysisOptions(
       content,
       sourceUrl: source.uri,
     );
-    validate(source, options);
+    validate(source, options, contextRoot: contextRoot);
   } on OptionsFormatException catch (e) {
     SourceSpan span = e.span!;
     errors.add(
@@ -297,6 +298,7 @@ class OptionsFileValidator {
   OptionsFileValidator(
     this._source, {
     VersionConstraint? sdkVersionConstraint,
+    required String contextRoot,
     required bool isPrimarySource,
     required AnalysisOptionsProvider optionsProvider,
     required ResourceProvider resourceProvider,
@@ -313,7 +315,12 @@ class OptionsFileValidator {
            sdkVersionConstraint: sdkVersionConstraint,
            isPrimarySource: isPrimarySource,
          ),
-         _PluginsOptionsValidator(),
+         _PluginsOptionsValidator(
+           contextRoot: contextRoot,
+           filePath: _source.fullName,
+           isPrimarySource: isPrimarySource,
+           resourceProvider: resourceProvider,
+         ),
        ];
 
   List<Diagnostic> validate(YamlMap options) {
@@ -945,11 +952,38 @@ class _PluginsOptionsValidator extends OptionsValidator {
     AnalysisOptionsFile.pluginsOptions,
   );
 
+  final String _contextRoot;
+
+  final String _filePath;
+
+  final bool _isPrimarySource;
+
+  final ResourceProvider _resourceProvider;
+
+  _PluginsOptionsValidator({
+    required String contextRoot,
+    required String filePath,
+    required bool isPrimarySource,
+    required ResourceProvider resourceProvider,
+  }) : _contextRoot = contextRoot,
+       _filePath = filePath,
+       _isPrimarySource = isPrimarySource,
+       _resourceProvider = resourceProvider;
+
   @override
   void validate(DiagnosticReporter reporter, YamlMap options) {
     var plugins = options.valueAt(AnalysisOptionsFile.plugins);
     switch (plugins) {
       case YamlMap():
+        var sourceDir = _resourceProvider.pathContext.dirname(_filePath);
+        var isAtContextRoot = sourceDir == _contextRoot;
+        if (!isAtContextRoot && _isPrimarySource) {
+          reporter.atSourceSpan(
+            plugins.span,
+            AnalysisOptionsWarningCode.pluginsInInnerOptions,
+            arguments: [_contextRoot],
+          );
+        }
         plugins.nodes.forEach((pluginName, pluginValue) {
           if (pluginName is! String) {
             return;

pkg/analyzer/lib/src/diagnostic/diagnostic_code_values.g.dart

@@ -40,6 +40,7 @@ const List<DiagnosticCode> diagnosticCodeValues = [
   AnalysisOptionsWarningCode.invalidOption,
   AnalysisOptionsWarningCode.invalidSectionFormat,
   AnalysisOptionsWarningCode.multiplePlugins,
+  AnalysisOptionsWarningCode.pluginsInInnerOptions,
   AnalysisOptionsWarningCode.recursiveIncludeFile,
   AnalysisOptionsWarningCode.removedLint,
   AnalysisOptionsWarningCode.replacedLint,

pkg/analyzer/messages.yaml

@@ -201,6 +201,15 @@ AnalysisOptionsWarningCode:
     hasPublishedDocs: false
     comment: |-
       An error code indicating multiple plugins have been specified as enabled.
+  PLUGINS_IN_INNER_OPTIONS:
+    parameters:
+      String contextRoot: the root of the analysis context
+    problemMessage: "Plugins can only be specified in the root of a pub workspace or the root of a package that isn't in a workspace."
+    correctionMessage: "Try specifying plugins in an analysis options file at '#contextRoot'."
+    hasPublishedDocs: false
+    comment: |-
+      An error code indicating plugins have been specified in an "inner"
+      analysis options file.
   RECURSIVE_INCLUDE_FILE:
     parameters:
       Object p0: the URI of the file to be included

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

@@ -78,6 +78,7 @@ class OptionsFileValidatorTest
   late final OptionsFileValidator validator = OptionsFileValidator(
     TestSource(),
     isPrimarySource: true,
+    contextRoot: '/',
     optionsProvider: optionsProvider,
     resourceProvider: resourceProvider,
     sourceFactory: SourceFactory([ResourceUriResolver(resourceProvider)]),
@@ -839,6 +840,57 @@ analyzer:
     );
   }
 
+  test_pluginsInInnerOptions() {
+    var code = '''
+plugins:
+  one: ^1.0.0
+''';
+    var filePath = '/inner/analysis_options.yaml';
+    newFile(filePath, code);
+    newFile('/pubspec.yaml', '''
+name: test
+version: 0.0.1
+''');
+    var diagnostics = analyzeAnalysisOptions(
+      sourceFactory.forUri2(toUri(filePath))!,
+      code,
+      sourceFactory,
+      '/',
+      null /*sdkVersionConstraint*/,
+      resourceProvider,
+    );
+
+    assertErrorsInList(diagnostics, [
+      error(AnalysisOptionsWarningCode.pluginsInInnerOptions, 11, 12),
+    ]);
+  }
+
+  test_pluginsInInnerOptions_included() {
+    newFile(optionsFilePath, '''
+plugins:
+  one: ^1.0.0
+''');
+    var code = '''
+include: ../analysis_options.yaml
+''';
+    var filePath = '/inner/analysis_options.yaml';
+    newFile(filePath, code);
+    newFile('/pubspec.yaml', '''
+name: test
+version: 0.0.1
+''');
+    var diagnostics = analyzeAnalysisOptions(
+      sourceFactory.forUri2(toUri(filePath))!,
+      code,
+      sourceFactory,
+      '/',
+      null /*sdkVersionConstraint*/,
+      resourceProvider,
+    );
+
+    assertErrorsInList(diagnostics, []);
+  }
+
   List<Diagnostic> validate(String code, List<DiagnosticCode> expected) {
     newFile(optionsFilePath, code);
     var diagnostics = analyzeAnalysisOptions(