This also includes some small refactorings for parameter names to start using the diagnostic wording instead of error. As well as a small change to `contextMessage` and related to use a list of `Pattern`s instead of a single `String` for better matching.
In future CLs, the TODOs added in this one will be addressed so we can fully migrate this. Added them because of the size of the CL.
Bug: https://github.com/dart-lang/sdk/issues/61557
Change-Id: I5557668a1baed98faff977255c9f22e0d9d96008
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/452180
Commit-Queue: Paul Berry <paulberry@google.com>
Auto-Submit: Felipe Morschel <git@fmorschel.dev>
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Work towards https://github.com/dart-lang/sdk/issues/61597
The analyzer_testing package's mock packages are "data" or "resource"
files which are notoriously hard to find on disk. No dart tooling I
know of can give me "the root directory of a package which is
referenced in a Dart program via "package:". Konstantin notes that
a program may be compiled ahead of time, and so the running VM itself
does not know where source files originally came from.
When users of the analyzer_testing package try to use
`addFlutterPackageDep` in a test, which attempts to locate the Dart
SDK's `pkg/` root directory, they get a StateError. This CL adds more
information and context to that error.
Change-Id: I3ae55cd580c8fa16ca86d66e6f7b655b8f9bcc42
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/452532
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This reverts commit 3923958595.
Reason for revert: Broke dart->flutter roll https://github.com/flutter/flutter/pull/176234 due to customer testing
Original change's description:
> [analyzer] Add experimental_member_use warning.
>
> Adds the implementation of the `experimental_member_use` warning
> (whose diagnostic code was introduced in
> https://dart-review.googlesource.com/c/sdk/+/451341).
>
> There is very little implementation logic, since the bulk of the
> implementation is shared with the existing `deprecated_member_use`
> warning. Unit tests are adapted from the `deprecated_member_use` unit
> tests.
>
> Since the tests in `non_const_argument_for_const_parameter_test.dart`
> use the annotation `@mustBeConst`, which is marked `@experimental`,
> they had to be modified to expect the new warning.
>
> Change-Id: I6a6a69645bb072b29166b7617b480a890b2bb5aa
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/450970
> Reviewed-by: Samuel Rawlins <srawlins@google.com>
> Commit-Queue: Paul Berry <paulberry@google.com>
Change-Id: I6a6a696447ed61b133e17ffc63ca6067fdbcc1ca
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/452526
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Adds the implementation of the `experimental_member_use` warning
(whose diagnostic code was introduced in
https://dart-review.googlesource.com/c/sdk/+/451341).
There is very little implementation logic, since the bulk of the
implementation is shared with the existing `deprecated_member_use`
warning. Unit tests are adapted from the `deprecated_member_use` unit
tests.
Since the tests in `non_const_argument_for_const_parameter_test.dart`
use the annotation `@mustBeConst`, which is marked `@experimental`,
they had to be modified to expect the new warning.
Change-Id: I6a6a69645bb072b29166b7617b480a890b2bb5aa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/450970
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change is only meant to improve consistency and readability in the test code. The `analysisOptionsContent` function had more parameters and now both take the same things. Also, `createAnalysisOptionsFile` now doesn't implement the same behaviour as `analysisOptionsContent` because it simply invokes it instead.
Change-Id: Ie84f30f4affc1fb106598d636cd6fcf263e6520e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445100
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Auto-Submit: Felipe Morschel <git@fmorschel.dev>
This change was generated by the following process:
- The script `pkg/analyzer/tool/messages/rename_error_constants.dart`
was run. This generated the vast majority of the diffs.
- Then all modified files were reformatted using
`tools/sdk/dart-sdk/bin/dart/format`.
- Finally, the script `pkg/analyzer/tool/messages/generate.dart` was
run, to rebuild generated code.
Change-Id: I6a6a69644ed8740ad6269d98cb169076151824ed
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444921
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Until recently, everything in rule_context.dart was found in
linter.dart. That file was broken up [1]. That file previously had a
lot of exports; it was a sort of barrel file for lint rules. In order
to make that change "minimal," (it was still huge), I retained some of
the exports. Now we're in the process of removing those exports.
In this change, I remove the exports for `rule_state.dart` and `linter_visitor.dart`. This doesn't affect too many files.
[1]: https://github.com/dart-lang/sdk/commit/438b560e44659cebf73bccba3ecf968806477c5c
Change-Id: I83110b22a07186560f6c853f033e070ac050ecd1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445123
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
The following classes are moved from `package:analyzer` to
`package:_fe_analyzer_shared`:
- `Diagnostic`
- `DiagnosticMessage`
- `DiagnosticMessageImpl`
The following declarations are also moved, since they are needed by
the above classes:
- `formatList`
- `Severity`
- `Source`
- `TimestampedData`
There is no change to the analyzer public API, and `export`
declarations have been added to the analyzer libraries that the
declarations have been moved from, so that code depending on these
declarations is unaffected.
These changes are part of a larger arc of work that introduces methods
`.withArguments` and `.at`, forming a literate API for reporting
analyzer errors that looks roughly like this:
diagnosticReporter.reportError(
ERROR_CODE.withArguments(...arguments...).at(...location...));
Moving this code into `_fe_analyzer_shared` is necessary because
scanner error codes are defined inside `_fe_analyzer_shared` (to allow
the scanner to be shared between the analyzer and CFE). Hence, to
avoid a circular depedency between `_fe_analyzer_shared` and
`analyzer`, the `.withArguments` and `.at` methods will need to live
in `_fe_analyzer_shared` too, as well as the classes representing the
diagnostic messages they create.
Note that there are some minor changes to
`pkg/analysis_server_plugin/api.txt` and
`pkg/analyzer_plugin/api.txt`; these have to do with the way the
`api.txt` generator chooses to report referenced elements, and don't
reflect actual API changes.
Change-Id: I6a6a6964a5c46f4a0205ce0d85620669ce55eb3c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444620
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@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>
The Spelunker class is only used by analyzer_testing's
PubPackageResolutionTest class, and by a utility script. So I believe
the best place for this class is in analyzer_testing, for two reasons:
* Would we also move the utility script
(`pkg/linter/tool/spelunk.dart`)? This is a script that lets you see
a visual tree of the syntax nodes of a Dart script. It has been
helpful to people writing lint rules, as it helps you understand how
you need to walk up or down the tree to check conditions. Therefore,
it will be at least as helpful to people writing analysis rules in
analyzer plugins (the primary consumers of PubPackageResolutionTest).
It doesn't need to live in `bin` (though that's one possibility). It
can live in `analyzer_testing/tool`.
* Then if the utility script lives in analyzer_testing, and
PubPackageResolutionTest lives in analyzer_testing, and these are the
sole consumers of Spelunker, it makes sense to move it into
analyzer_testing. It does not need to be public API; just live in the
source code.
Change-Id: Id607091b35ab83c96b8cd73f0ece63923c934fb4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444240
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
The feature will go away when we get any new part-affecting
language feature, whether enhanced parts or import shorthands.
Might as well be prepared.
Not touching tools/dom.
CoreLibraryReviewExempt: No changes to actual code. Also, vacation.
Tested: No behavior change, no new tests.
Change-Id: Ie70f07b9f8f73edd93bf37b93de60662e271a0e2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/440221
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This fixes a bootstrapping problem: previously we could get into a
situation where the generated files for analyzer diagnostic messages
were out of date, but the code generator for those messages couldn't
be run, because it imported the analyzer itself, which contained
compiled errors due to the generated files being out of date.
There were two problematic dependencies:
- The `GeneratedContent.check` and `GeneratedContent.checkAll` methods
relied on invoking the Dart formatter as a library (rather than
invoking `dart format` as a subprocess); the Dart formatter library
imports the analyzer. Fortunately, this functionality is not needed
by the code generators themselves. It's only needed by the _tests_
that verify that code generation has run. So I was able to break the
dependency by moving this functionality into a new file,
`package:analyzer_utilities/generated_content_check.dart`, which is
imported only by the code generation tests. The new API is slightly
different: `GeneratedContent.checkAll` has been replaced by an
extension method `check` on `Iterable<GeneratedContent>`.
- The diagnostic message code generator used to have a post-processing
step that made use of the scanner to find error messages used by the
parser that needed to be translated, and issued a warning if it
found any. This check was useful a long time ago, when we were
unifying the analyzer and CFE parsers, and is no longer useful, so
I've removed it.
Change-Id: I509b741faca3b9ae21c23936001105ded731b490
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442241
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Modifies `GeneratedDirectory.outputDirPath` and
`GeneratedFile.outputPath` to be relative to the SDK's `pkg` directory
rather than relative to the containing package. Accordingly, modifies
the `GeneratedContent` methods `check`, `checkAll`, `generate`,
`generateAll`, `output`, as well as the `DirectoryContentsComputer`
and `FileContentsComputer` callbacks, so that their first parameter is
the path to the `pkg` directory rather than the path to the containing
package.
Also modifies the `readApi` functions in `pkg/analysis_server` and
`pkg/analyzer_plugin` to accept a path to the `pkg` directory rather
than a path to the containing package, since these functions are
called by code generation callbacks.
These changes should make code generation logic easier to reason
about. They also will make it easier to move the outputs of code
generation from one package to another, which will pave the way for
some follow-up work in which I intend to start sharing error message
representations belonging to `pkg/analyzer`, `pkg/front_end`, and
`pkg/_fe_analyzer_shared`.
Change-Id: Ia9b369b16f2df931c8a472f91400f2c5a0b8be9d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/438480
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change removes the top level variable `allTargets` from
`pkg/analyzer_utilities/lib/tool/api.dart` and replaces it with a
method `allTargetsForPackage`, parameterized by the name of the
package for which an `api.txt` file is generated. The individual code
generators that make use of this function now all have their own top
level variable `allTargets`.
In addition to being conceptually clearer (since we no longer have a
single `allTargets` variable with different meanings depending on
where it's used), this paves the way for a CL I am working on that
will change the `GeneratedContent` based code generators so that they
use paths relative to the `pkg` directory. That in turn should make it
simpler to move some of the code generated files that have to do with
error messages from `pkg/analyzer` to `pkg/_fe_analyzer_shared`.
Change-Id: Id636ddbca27c46ae4242bbf8e4b6f7b8dae4d4e9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/438481
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Auto-Submit: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This CL removes the unnecessary overrides of the `experiments` getter
from the tests and collects the corresponding experiment flags added to
the returned value of the getter in the individual tests to the
'experimentsForTests' list that is used by all tests. Specifically, the
experiments `variance` is enabled for all tests.
The flag `inference_update_1` is removed from the experiments list,
since it's enabled by default. In the tests that explicitly require a
distinction between the flag being enabled and disabled, the getter
`_isEnabled` is overridden and returns a value corresponding to the
test.
The flag `inference_update_4` still appears in one of the overrides of
the `experiments` getter, since enabling it in all tests appears
breaking.
This is a follow-up to
https://dart-review.googlesource.com/c/sdk/+/430720/comment/4c3f016e_4933f925/
Change-Id: I4d35e3c6259e2f7e6751d180b5790c13f68de1ba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/434360
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@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>
UNUSED_ELEMENT and UNUSED_FIELD were added to the "ignore list" for
the tests of a dozen lint rules. I think it is pretty harmless to ignore
them by default, and save plugin authors some `// ignore:` comments, or
hand-wringing about how to get rid of an "unused" field in a test which
is needed in order to test an analysis rule.
Change-Id: Ic35fead8a44d8d3341f9e32e92b53610b517edf5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/430121
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Most of the behavior of LintRuleTest is extracted into a public class
in analyzer_testing, AnalysisRuleTest. A little bit of code is left in
LintRuleTest, so that first-party lint rules can be tested by
registering the first-party lint rules. And those lint rules can
declare the `lintRule` that each is concerned with, rather than the
new `analysisRule` getter.
PubPackageResolutionTest is left as private impl, because I think it
may undergo some changes, and we may be merging more shared code, and
the name may change. A few first-party lint rules have tests that
refer to this class, which is OK.
Other than that, no code or behavior changes are in this CL.
Work towards https://github.com/dart-lang/sdk/issues/55660
Change-Id: I447850c438af7d0fe90889d2eaec6f2f8b4f8281
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429420
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Konstantin Shcheglov