Skip to content

Request: ability to configure the analysis server #23851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sethladd opened this issue Jul 15, 2015 · 8 comments
Closed

Request: ability to configure the analysis server #23851

sethladd opened this issue Jul 15, 2015 · 8 comments
Assignees
Labels
devexp-server Issues related to some aspect of the analysis server legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Milestone

Comments

@sethladd
Copy link
Contributor

We'd like to be able to configure the analysis server, in such as way that the configuration is associated/tied to a project.

There are a few things we'd like to configure, among other things:

  • which directories should be ignored during analysis (blacklist)
  • which warnings/hints should be displayed to a user
  • a whitelist of directories to analyze (potentially)
  • plugins (potentially)

Ideally, this configuration is human readable/writable, is machine readable/writable, is easy to extend, can be checked into source control, and is discoverable by the analysis server.

@sethladd sethladd added Type-Enhancement legacy-area-analyzer Use area-devexp instead. devexp-server Issues related to some aspect of the analysis server labels Jul 15, 2015
@sethladd
Copy link
Contributor Author

Initial thoughts:

  • a dot file lives at the root of the project
  • YAML
  • discovered by the analysis server automatically

@pq
Copy link
Member

pq commented Jul 16, 2015

Here's a sample of the kind of format we've been discussing:

analyzer:
  ignore:
    - lib/foo.dart
    - 'test/data/*'
  disable:
    - error_id1
    - error_id2
linter:
  include: '**/src'
  rules:
    camel_case_types: true # shorthand for enabled: true
    new_lines:
      - enabled: true
      - chars: '\n'

Linter config ideas derived from #57189 and included just for context. Consuming linter options will be the province of the linter.

The most salient bit is the idea that it's YAML and has a mapping from 'scopes' to options.

For further reference, here's where we parse options in analyzer_cli:
https://github.com/dart-lang/analyzer_cli/blob/master/lib/src/options.dart#L493 (look for OptionsFileParser)

And here's where the options format is defined for plugins:
https://github.com/dart-lang/sdk/blob/master/pkg/analyzer/lib/plugin/options.dart

Regarding discovery by server, I think we should allow options to live at all "context roots" (e.g., next to a pubspec or .packages spec). How about .analysis_options as the magic file name?

@seaneagan
Copy link

Maybe the file should be read by dartanalyzer as well?

@bwilkerson
Copy link
Member

Maybe the file should be read by dartanalyzeras well?

Longer term we plan on allowing dartanalyzer to analyze a directory structure, at which point, yes.

But as it stands today, I don't think it's useful to run dartanalyzer myFile.dart and have it not analyze the file because the file is marked as being excluded.

Just to be clear, this won't prevent analyzer from analyzing a file that is referenced from a non-ignored file. The semantics are more accurately described as: don't analyze any file that matches one of the given patterns unless it's necessary in order to analyze a file that doesn't match one of the given files.

@bwilkerson
Copy link
Member

By the way, the previous comment would have been easier to write if the second-level key ('ignore') were instead 'exclude' (because 'exclude' has a nice opposite -- 'include' -- while 'ignore' doesn't, unless it's something like 'attend', but 'attended files' just doesn't sound right).

@sethladd
Copy link
Contributor Author

Regarding discovery by server, I think we should allow options to live at all "context roots" (e.g., next to a pubspec or .packages spec). How about .analysis_options as the magic file name?

sgtm

(also, bumping to High because it's really important for our Sky customers, and we're starting on it anyway :)

@sethladd sethladd added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed Priority-Medium labels Jul 16, 2015
@pq
Copy link
Member

pq commented Jul 16, 2015

Maybe the file should be read by dartanalyzer as well?

With a CLI option it already sort of is (though no-one is doing anything with it). Interested plugins are all passed the contents for independent processing. Migrating the linter to use this mechanism is on the short-list. (And needless to say none of this include/exclude configuration logic is implemented yet either.) As Brian says, discovery will come down the road.

But as it stands today, I don't think it's useful to run dartanalyzer myFile.dart and have it not analyze the file because the file is marked as being excluded.

👍 More sensibly you would want lint (and other plugin) configurations to get picked up though I think.

By the way, the previous comment would have been easier to write if the second-level key ('ignore') were instead 'exclude'

I noticed that as I was putting together this proposal. If we also want an opposite (and I don't see why we should rule it out), let's go with include.

Any objections?

@sethladd
Copy link
Contributor Author

Initial support is done. Analysis Service now reads a .analysis_config YAML file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-server Issues related to some aspect of the analysis server legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants