I had not run the first fix through manual testing. Manual testing
revealed a second location (!!) where the file path is stored, for
diagnostics. This updates the code to send a notification for each file
path, including parts.
Change-Id: Iee60330c579563f31c8d9a2c65a22c9cde4b64d8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448881
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Fixes https://github.com/dart-lang/sdk/issues/61449
The previous code, sure enough, only created a DiagnosticReporter for
the defining unit, and only visited the main unit.
The fix is to create a DiagnosticListener for each unit, set the
DiagnosticReporters correctly and the "current unit" as we go, and then
pull the reported diagnostics from each listener.
Change-Id: I6b645284c58e5e35ccc5b89f30f6ffe97f48296c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448644
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This renames the old screen, "Legacy plugins" and adds a new screen,
"Plugins". On the new plugins screen, we ask the plugins isolate(s) for
their plugins details, and print the following, for eadh plugin:
* the plugin's name
* the names of the registered lint rules
* the names of the registered warning rules
* the IDs and "messages" of the registered assists
* the IDs and "messages" and associated diagnostic codes of the
registered quick fixes
More to come in follow ups:
* The resolved versions of plugin packages (coming from package_config.json)
Change-Id: Ic3dc4c5bffa64fd4da4097c042a847cc064e41ce
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447763
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This week we moved some other "plugin analysis options" code into the
private API. This continues that work, in order to add plugin source
options into the AnalysisOptionsImpl signature code.
The classes are deprecated as far as the public API is concerned,
encoded with a `@Deprecated` export. This is not reflected in api.txt.
Change-Id: Ie95851a7b0d54ef6abb8fb1e2b41294fbc5db836
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447841
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This change adds a new parameter `typeParametersInScope` to `DartEditBuilder.writeType`, and related methods to deprecate the previous `methodBeingCopied` parameter, which limited our ability to pass in some type parameters that are in scope.
Also fixes a TODO about testing if the `Record` feature is enabled or not.
And as a bonus, we now always write down type parameters by looking for the closest public type or falling back to `Object?` or `dynamic`.
Bug: https://github.com/dart-lang/sdk/issues/61186
Change-Id: I7a7b1c62adb81aecfdf61ebbaa0d95f1d33bced8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442282
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Auto-Submit: Felipe Morschel <git@fmorschel.dev>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
This library mostly exports _public_ API, and the to-be-deprecated
NodeLintRegistry class. So we just move private RuleContext classes
into a new library, and remove the exports.
The lint rules then are adjusted, mostly just renaming
NodeLintRegistry -> RuleVisitorRegistry. But also, this removes a lot
of implementation imports; most rules now have _no_ private imports!
Change-Id: I023f7efa979f94d208a6ae7421902145e8761c9a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443364
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Because ChangeBuilder can be used to modify multiple files, the EOL should come from an individual files edit builder and not from the overall ChangeBuilder.
This change renames `eol` on ChangeBuilder to `defaultEol` (since it's still useful to supply a custom default, for example when moving a class from an existing file to a new file, we want to default to the same EOL as the source file), and adds an EOL to file edit builders which is computed from the file content (and falls back to the default from ChangeBuilder if there are none).
My intention here is to not introduce any breaking changes (yet) so the ChangeBuilder constructor interface still has the "eol" named parameter (marked as deprecated) as well as the new `defaultEol` parameter.
I added a new `StringExtension` for `endOfLine` to both analyzer_plugin and analysis_server_plugin as we need this in both places (but I expect one of those projects will ultimately be dropped). This slightly differs from `CorrectionUtils.endOfLine` because it can return `null` which is a signal to fall back to the ChangeBuilder default instead of forcing its own default. (CorrectionUtils.endOfLine now uses that extension, although because of the null/default, it's likely that some - if not all - uses of this should migrate to the new extension and fall back to a default from a ChangeBuilder).
Change-Id: Ie2e6eaba78c96a1cbd79eba881e12bbeef724aa8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443122
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Before an assist call on a file with non-windows lineendings would first
search through the entire file for Windows line endings, then search for
the non-Windows line endings. As such Windows line endings was faster.
This CL changes the semantics slightly from "Windows line endings if
any Windows line endings exist" to "whatever line endings are first
used" which should be good.
It speeds up assist calls with non-Windows line endings:
Statistics on 5 runs each:
```
Windows / 2000: No change.
Windows / 4000: No change.
Windows / 8000: No change.
Unix / 2000: 1000 assist calls: -46.9900% +/- 6.1715% (-0.88 +/- 0.12) (1.86 -> 0.99)
Unix / 4000: 1000 assist calls: -59.1561% +/- 6.3982% (-1.79 +/- 0.19) (3.03 -> 1.24)
Unix / 8000: 1000 assist calls: -63.8429% +/- 4.3083% (-3.68 +/- 0.25)(5.76 -> 2.08)
```
Change-Id: I61278ed4ad31b891e87951da0a6162fe37376888
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/437224
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
The diff makes this change look bigger than it is.
Here's how the code worked before: `compute()` called `_addFromProducers()`. `_addFromProducers()` had a local function, also
called `compute()`, which is called per CorrectionProducer. The local
`compute()` function would unconditionally create a ChangeBuilder and
call each CorrectionProducer's `compute()` method, then call back out
to an instance method called `_addFixFromBuilder()`, which would drop
the computed change on the floor if `fixKind` was `null`.
This CL contains the following changes to the above system:
* The local `compute()` function and the instance method,
`_addFixFromBuilder()` are combined together, into one instance
method, `_addFromProducer()`.
* Then, the `fixKind` check is moved to the very top of that
method, bailing out _before_ computing changes if `fixKind == null`.
Change-Id: I6c61059fc36ec1587b15ca01b43830a5b179da9a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/417326
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Work towards #60635
In this change, we rework the AnalysisErrorListener deprecation to
better support users who have their own class that implements
AnalysisErrorListener. This change introduces a sealed supertype,
DiagnosticOrErrorListener, with the old implementation,
AnalysisErrorListener, and the new implementation, DiagnosticListener,
as its sole direct subclasses. Users who have implemented
AnalysisErrorListener should be able to instead implement
DiagnosticListener, and their class is an acceptable instance of
DiagnosticOrErrorListener, wherever that is needed.
In a breaking change we can drop AnalysisErrorListener and deprecate
DiagnosticOrErrorListener, and in the next breaking change, we can drop
DiagnosticOrErrorListener.
For reference, see the first API difference when deprecating AnalysisErrorListener and introducing DiagnosticListener: https://github.com/dart-lang/sdk/commit/903d77cc8229972a424941dcb7b7b79741e833eb#diff-dec15868961d7eadcd009f49d129bebcdb747aaa8dbfde5f5a08884e0cf11e32
Change-Id: I3ccf11d54b41fbca98d020d89978d250c16b4c04
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/436480
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Work towards https://github.com/dart-lang/sdk/issues/60635
I tried to keep this minimal but still sensible:
* Rename AnalysisErrorListener and all subtypes to use 'Diagnostic'.
* Rename all instantiations of such classes if they previously
contained the word 'error'.
* Rename `RecordingDiagnosticListener.errors` to `.diagnostics`.
* Rename some _testing_ instance members that had the word 'error'
to instead use 'diagnostic'.
Change-Id: I3948e27ba28ac2494092e04f4e1d201a20cc1135
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/433004
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This is a big CL; no code is really "changed." We basically move 3
components into the public API, which can all be reviewed concisely
in the `api.txt` file.
* The AnalysisRule classes: `AbstractAnalysisRule` (which is good to
make public as a lot of the public API is specified and documented
here), `AnalysisRule`, `MultiAnalysisRule`.
* The Pubspec classes, available for linting pubspec files:
`PubspecDependency`, `PubspecDependencyList`, `PubspecEntry`,
`PubspecEnvironment`, `PubspecGitRepo`, `PubspecHost`, `PubspecNode`,
`PubspecNodeList`, `PubspecVisitor`.
* The `RuleVisitorRegistry` class. This class is needed by analysis
rule authors, and is part of the public API of AnalysisRule.
Change-Id: Ib1803180de9469f4ff39cf1778f96787f5f74b14
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432363
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
For the analyzer plugin APIs, these classes should not have the word
'Lint' in them, as they are used for all analysis rules, whether they
report lints or warnings.
* LinterContext -> RuleContext
* LinterContextWithParsedResults -> RuleContextWithParsedResults
* LinterContextWithResolvedResults -> RuleContextWithResolvedResults
* LintRuleUnitContext -> RuleUnitContext
I'm open to better names for these. Some of these suffer from a
chain-of-nouns name (a "RuleUnitContext" is a Context for a Unit for
a Rule). So we could go with "UnitContextForRule" and "ContextForRule"
or other ideas like that.
Only RuleContext and RuleUnitContext will be public API.
Change-Id: I5550736beaba7e39b36e7b5584ca6d023aa4d02b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/431703
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Copying change builders in order to enable reverting changes from a
single correction producer when there's an exception is expensive.
This CL replaces that implementation with a transactional model.
Clients are not required to start a transaction (that happens
automatically), but are required to signal the end of a transaction by
invoking either `commit` or `revert`. (Actually, `commit` is assumed
if neither method is invoked before computing the `SourceChange`.)
There is some information in the Dart file edit builder related to
imports that isn't correctly handled. The reason for this is that too
much of the import computation is done up-front. I don't think this
will be a problem in practice (or at least not often), but we should
come back at some point to change the implementation so that we retain
abstract data longer and perform more processing at the very end (where
we'll have complete information and probably be able to do a better
job anyway).
This CL improved the performance of the benchmark that adds and applies
fixes for 10,000 lint violations.
Before it took an average of 11216.0 ms.
After it took an average of 4200.2 ms.
Which is about a 62% improvement.
Change-Id: I12710606455a04e34a82308ede5d9fba1c68b972
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428060
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Before this change, we had an enforcement that either 'get lintCode' or
'get lintCodes' is implemented, but it was a runtime enforcement. And it
was a little roundabout.
The new system is simpler and more typical:
* `AbstractLintRule` is the parent type, and it's sealed. It provides
`get lintCodes` as an interface. Code outside of the lint rules
requires this getter, in registering codes, etc.
* `LintRule` is the new subclass which is the "90% case." It declares
a `get lintRule`, and offers simpler reporting methods for reporting
a single code everywhere.
* `MultiLintRule` is the new subclass which represents that last
"10% case." It only declares the reporting methods which all require
a LintCode parameter, since there is no concept of a "default lint
code among all my lint codes."
Work towards https://github.com/dart-lang/sdk/issues/50986
Change-Id: I63cbbfc1d936bb4428aa0b8f5fb0638c9b61b3dc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426284
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This allows us to traverse the children without needing to create lists.
It also allows us to iterate over lists of nodes more efficiently.
Tested against the script that computes fixes for 10,000 diagnostics,
I'm seeing the following result.
Before this CL the average time was 10692.4 ms.
After this CL the average time was 1900.0 ms.
That's just over an 82% improvement.
Change-Id: Ideff745288c8990e9948b22c5204d176af3cac6e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426904
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Sam Rawlins