Previously, in the `CheckSorted` presubmit check, if
`verify_sorted_test.dart` returned a nonzero exit code but none of its
output lines contained the string `Unsorted file`, then a presubmit
failure would be generated containing an empty string, resulting in
confusing output like this:
Running presubmit upload checks ...
18.4s to run CheckChangeOnUpload from /home/paulberry/dart1/sdk/pkg/_fe_analyzer_shared/PRESUBMIT.py.
** Presubmit ERRORS: 1 **
Presubmit checks took 21.4s to calculate.
There were presubmit errors.
This might happen, for example, if `verify_sorted_test.dart` contained
a compile-time error or threw an unhandled exception.
With this change, the message is `CheckSorted: could not parse output
of verify_sorted_test.dart`, followed by the full stdout and stderr
from the attempt.
This should make this sort of failure a lot easier to debug.
Note: I've made this same fix before to
`pkg/_fe_analyzer_shared/PRESUBMIT.py` (see
https://dart-review.googlesource.com/c/sdk/+/485601). I didn't realize
at the time that the code was duplicated.
Note: This should make it easier to debug
https://github.com/dart-lang/sdk/issues/63464.
Change-Id: I4892dcff7ad6969ca40564c1fd7e62e36a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/506605
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Replace the positional Scanner constructors with a single constructor
that takes the input text and error reporting callback as named required
arguments.
Remove the unused fasta forwarding constructor, reader offset handling,
and stored first token. The scanner now keeps only the input text and
returns the token stream directly from tokenize.
Update analyzer, analysis server, plugin, CLI, scrape, and scanner test
call sites to use the named constructor arguments.
Change-Id: Ieabb7b28570d4a963563fb2d1c5ef5ac0c89032a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500340
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
The function mostly returned null/nonsense if given a null argument,
and the few call sites with nullable arguments (in tests and tools)
are easily tweaked.
Change-Id: I7f9993696c4bd822b648a5a89114e6af25eac552
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/490020
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This CL is part of an effort to bump the SDK requirement to `3.12.0-0`
for all the packages in `pkg` that are not published to `pub`, so that
we can get better testing of the "private named parameters" feature.
(Packages that *are* published to `pub` can't be safely bumped yet,
because SDK 3.12 hasn't been released, and I don't want to block those
packages' ability to publish useful updates to customers.)
This change covers the following packages, which are owned by
OWNERS_MODEL:
- pkg/analyzer_cli
- pkg/frontend_server
- pkg/testing
Changes to `pubspec.yaml` files were made manually.
Changes to `.dart` files were made automatically, using `dart fix` to
fix failures in the unnecessary_underscores lint.
Change-Id: Ie444a97b2aed3f9eabfde4963c19cfb26a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/487883
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Auto-Submit: Paul Berry <paulberry@google.com>
Replaces the top level function `analyzeAnalysisOptions`, which
contained 3 nested local functions, with a class
`AnalyzeAnalysisOptions`, where the nested local functions have been
changed into private methods.
All the variables in the top level function that were implicitly
write- or read-captured by the nested local functions have been
changed into class fields or explicit method parameters. This should
make the code a lot easier to follow.
In this CL, I've tried hard to preserve the existing functionality
exactly; besides converting local variables to fields and parameters,
I haven't really made any changes to the core logic beyond code
motion. In follow-up CLs, I plan to make some simplifications that are
less obviously semantics preserving (but should be easier to review
since they won't involve so much code motion). Once that is done I
hope to be able to migrate the error reporting in this class to use
the literate diagnostic API.
Change-Id: I6a6a696414ecae80535c8d6633eaa567d37dcccc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/479080
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Remove the analyzer-only `scanner/reader.dart` re-export and refactor
`Scanner` to accept source text directly.
* Delete `pkg/analyzer/src/dart/scanner/reader.dart`, which only re-exported
`_fe_analyzer_shared` reader types.
* Replace the `Scanner(CharacterReader, ...)` factory with
`Scanner(String contents, ...)`, aligning the API with the underlying
`scanString` implementation.
* Update analyzer, analysis_server, analyzer_cli, analyzer_plugin, and scrape
call sites to pass the source string directly instead of constructing
`CharSequenceReader`.
This removes an unnecessary abstraction layer, reduces
imports/indirection, and makes scanner usage simpler and more uniform
across the repo.
Change-Id: I380b365676c8a62971a3700deadb695d1519d653
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/477600
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
I found one instance of a variable called "packagesFile" and well, I
pulled that thread :/
There is a nice functional simplification here: The `_findPackagesFile`
function returned a `_PackagesFile` object, which contained both a
`Folder parent` and a `File file`. The latter was never referenced, so
simplification 1. Also, the reason to have an object that stores both
is that this object handled both `.packages` files, whose immediate
parent is `parent`, and `package_config.json` files, whose _grandparent_
is `parent`. But with the distinction gone, we don't need this
`_PackagesFile` class, and `_findPackagesFile` can instead return the
folder, and be renamed `_findFolderWithPackageConfigFile`.
Change-Id: Id7cfad02bad16baeebfd8a719a3a92187fec730f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/475930
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Changes the signature of the following constructors:
- Scanner
- Scanner.fasta
- Parser
So that they accept a `DiagnosticReporter` object rather than a
`DiagnosticListener` object. This brings the scanner and the parser
into alignment with the majority of the rest of the analyzer (which
reports errors using `DiagnosticReporter` rather than by talking to
`DiagnosticListener` directly).
It also makes the `source` parameter of these constructors
unnecessary, because the `source` can be obtained from the
`DiagnosticReporter`.
Change-Id: I6a6a6964607ffb52a8332a6e618dc0c9a1e48c5c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/473442
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Changes the logic in `pkg/analyzer_cli` to use
`DiagnosticCode.lowerCaseName` instead of `DiagnosticCode.name`, and
`DiagnosticCode.lowerCaseUniqueName` instead of
`DiagnosticCode.uniqueName`. This ensures that diagnostic codes are
matched in a case-insensitive fashion.
This paves the way for deprecating (and eventually removing) the
`DiagnosticCode.name` and `DiagnosticCode.uniqueName` getters.
Change-Id: I6a6a6964f1254f248d2b772f69e33adf0b682bb9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/466186
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Adds the getters `lowerCaseName` and `lowerCaseUniqueName` to the
`DiagnosticCode` class. These getters behave the same as the `name`
and `uniqueName` getters, except that they convert the string to lower
case before returning. These getters should help make it easier for
the analyzer and related packages (as well as analyzer clients) to
treat diagnostic codes in a case-insensitive fashion.
In follow-up CLs, I plan to switch the analyzer and related packages
over to using the new getters, and then I'll deprecate and eventually
remove `name` and `uniqueName` from the analyzer public API.
Change-Id: I6a6a6964e8891fc6f74a9777eae375f1a553c2f3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/466182
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Removes the field `AnalyzerImpl.options`, the `options` parameter of
the method `Driver._runAnalyzer`, and the `commandLineOptions`
parameters of the functions `computeSeverity` and
`determineProcessedSeverity`. None of these were used.
Change-Id: I6a6a6964617cecb81f0d1c6570be1d78fb599af4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/463085
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Changes the analyzer and related packages so that when they refer to
diagnostic constants, they do so via the import prefix `diag`, which
refers to the appropriate `diagnostic.dart` file containing the top
level diagnostic constant declarations, rather than the static
declarations inside `DiagnosticCode`-derived classes (which will soon
be removed).
This CL was created by the following steps:
- Run the script
`pkg/analyzer_utilities/tool/messages/switch_to_toplevel_diagnostics.dart`.
- Execute `dart fix --apply --code=unused_import,unnecessary_import`
on the following directories (this removes imports that are no
longer necessary due to the change):
- `pkg/analysis_server`
- `pkg/analyzer`
- `pkg/linter`
- `pkg/analysis_server_plugin`
- `pkg/analyzer_plugin`
- `pkg/analyzer_testing`
- `pkg/front_end`
- `pkg/analyzer_cli`
- Execute `dart format` on the following files and directories:
- `pkg/analysis_server`
- `pkg/analyzer`
- `pkg/linter`
- `pkg/analysis_server_plugin`
- `pkg/analyzer_plugin`
- `pkg/analyzer_testing`
- `pkg/front_end/test/scanner_test.dart`
(Note that `pkg/front_end` and `pkg/analyzer_cli` are not
re-formatted as whole directories because they contain `.dart` files
that are test cases rather than source code, and reformatting those
files might change test expectations.)
- Manually add `diag` to
pkg/front_end/test/spell_checking_list_tests.txt.
- Manually fix the ignore comment in
`pkg/analyzer_testing/lib/src/analysis_rule/pub_package_resolution.dart`. (The
script `switch_to_toplevel_diagnostics.dart` automatically adds it
after `import 'package:analyzer/src/diagnostic/diagnostic.dart' as
diag;`, but then executing `dart format` bumps the ignore comment to
the following line, where it has no effect.)
Change-Id: I6a6a69643022aab2b5a6224fb4124eead243260d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/461521
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Renames `pkg/analyzer/lib/src/diagnostic/diagnostic.dart` to
`pkg/analyzer/lib/src/diagnostic/diagnostic_message.dart`. The new
name aligns better with the library's purpose (which is solely to
export the class `DiagnosticMessageImpl` from
`package:_fe_analyzer_shared`).
This frees up the filename
`pkg/analyzer/lib/src/diagnostic/diagnostic.dart` to be used as the
repository for all generated analyzer messages.
Change-Id: I6a6a6964be0904a383334974f1e64a03c0df551b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/458381
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Not for performance, because it is one shot, but all language/ and co19/
code will go through manifests builder, and give us better coverage.
Change-Id: Iae83ec685084847358d960deff81d18fb9904f4f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/456404
Reviewed-by: Samuel Rawlins <srawlins@google.com>
The new updateAnalysisOptions4 parameter is a callback function,
similar to updateAnalysisOptions3, but with one removed parameter
itself, the `DartSdk sdk` parameter.
The one place where this parameter was used in the callback passed by
the caller, was in DAS's ContextManager, and it was just used in order
to compute a FeatureSet from (a) the computed DartSdk and (b) the
command-line `--enable-experiment` options. We can instead provide a
`enabledExperiments` parameter on AnalysisContextCollectionImpl,
leading to a simpler API and still a simple implementation in
ContextBuilder.
The reason for the incremental migration is that dartdoc, and possibly
some internal clients, use updateAnalysisOptions3.
Change-Id: Ie990ab8f7b850aed1ce3ef39ab71c01835c080da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447400
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
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>
* OptionsValidator was defined in the `src/plugin/` directory (the only
file in there), which I found confusing. Moved it to
`src/analysis_options/`.
* OptionsFileValidator, and a few related classes, were defined in the
`src/task/` (the last file in there!), so I moved it to the same
place.
There is more to tidy in here, but this is a simple file-move.
Change-Id: I8f4c5cc0ae4e76eb6c17249457c50ab69c82590d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443147
Reviewed-by: Konstantin Shcheglov <scheglov@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>
Work towards https://github.com/dart-lang/sdk/issues/55660
The comment for `MemoryResourceProvider.convertPath` includes:
> This is a utility method for testing; paths passed in to other
> methods in this class are never converted automatically.
and indeed, the actual impl of this method is found in an extension,
in analyzer's test_utilities/ directory. It seems to me better to
leave a testing utility as a testing utility, and not expose it in
the public API of MemoryResourceProvider.
Additionally, the extension method is used by `ResourceProviderMixin`,
which is moving to the public analyzer_testing package. It is illegal
to have a circular non-dev dependency between the analyzer package
and the analyzer_testing package.
The migration for the ~half dozen test files that use this method is
to call the extension method directly. Sometimes with an extension
override, and sometimes without.
Change-Id: I9c2e18600461134bfd91c082b3af0d5079600f0c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426984
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Work towards https://github.com/dart-lang/sdk/issues/60635
* `AnalysisError._contextMessages` unnecessarily backed the public
`contextMessages` getter; the field is final so it can be public
itself.
* `AnalysisError._correctionMessage` unnecessarily backed the public
`correctionMessage` getter; the field is final so it can be public
itself.
Change-Id: If269d4ed590ef7df81d9b9e3be03766601526d7f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425620
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Paul Berry