In https://dart-review.googlesource.com/c/sdk/+/399480, a mechanism
was introduced to `_fe_analyzer_shared` to support null-shorting.
Sometime later, in
https://dart-review.googlesource.com/c/sdk/+/427341, the CFE was
changed to use this shared mechanism.
This CL completes the arc of work by changing the analyzer to use the
new shared mechanism.
The public API class `NullShortableExpression`, which was part of the
analyzer's old implementation of null shorting, is no longer used. It
should never have been exposed through the analyzer public API in the
first place, so it's been deprecated; I will update the pending
changes for analyzer 9.0.0 to delete it.
Change-Id: Ic711df6c537c454aa8a99861135e63c8dd277485
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400021
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
The implementation isn't shared yet, but is extracted into the
implementations of `TypeAnalyzerOperations` class, to ensure the
internal dependencies on the other tool-specific functions and classes
is weakened. This is a preliminary step to sharing `chooseTypes`
between the Analyzer and the CFE.
Part of https://github.com/dart-lang/sdk/issues/54902
Change-Id: I4441a9b5dacffd6567ed7e5a3937f64a119fa10f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/446080
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
This CL enables the new literate API for reporting analyzer
diagnostics that was introduced in
https://dart-review.googlesource.com/c/sdk/+/445803.
The two CLs are separated for easier code review:
- https://dart-review.googlesource.com/c/sdk/+/445803 introduces the
necessary infrastructure classes and modifies the code generator,
but all code generator changes are initially disabled using the flag
`literateApiEnabled`.
- This CL flips the `literateApiEnabled` to `true`, regenerates the
generated code, and makes trivial adjustments to imports necessary
to support the newly generated code.
In follow-up CLs, I will transition the analyzer over to reporting
errors using the new API.
Change-Id: I6a6a69647946f19a370724dbd6f36e02ed0f1d61
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445783
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change introduces a new literate API for reporting analyzer
diagnostics. The new API looks like this:
diagnosticReporter.reportError(
diagnosticCode
.withArguments(...) // omitted if diagnostic takes no arguments
.withContextMessages(contextMessages) // may be omitted
.at(astNode),
);
For comparison, the old API looks like this:
diagnosticReporter.atNode(
astNode,
diagnosticCode,
arguments: [...], // omitted if diagnostic takes no arguments
contextMessages: contextMessages, // may be omitted
);
For the moment, this new API is internal to the analyzer; it is not
exposed through the analyzer public API. This is to give us time to
try it out and make changes if necessary before we have to commit to
it.
The advantages of the new API are:
- Better static type checking: with the old API, if we accidentally
forgot to supply arguments to a diagnostic code that required them,
or vice versa, or supplied the wrong number of arguments, the
mistake would not be caught until runtime. If we accidentally
supplied arguments of the wrong type, the mistake would not even be
caught at runtime. With the new API, any of these mistakes will lead
to a compile-time error.
- Better code completion support: with the old API, if we can't
remember whether a diagnostic code requires arguments, we have to
look it up. With the new API, we can type the diagnostic code
followed by `.`, and completions will be offered for either
`.withArguments` (if arguments are required) or
`.withContextMessages` and `.at` (if no arguments are
required). Furthermore, while typing inside the parentheses after
`.withArguments`, completion will offer the names of the required
arguments.
To allow for a gradual transition to the new API, the old API is still
supported. To make this possible, a new sealed class `Reportable` is
introduced, to act as the parameter type for
`DiagnosticReporter.reportError`. It has two derived classes:
- The existing `Diagnostic` class (which was the old parameter type
for `DiagnosticReporter.reportError`)
- A new `LocatedDiagnostic` class (which is the return type of the new
literate `at` method).
The difference between these two classes is that the `Diagnostic`
class has already had its arguments formatted and disambiguated using
`convertTypeNames`, whereas the `LocatedDiagnostic` class hasn't.
The only change to the analyzer public API for now is the introduction
of `Reportable` and the change to the type signature of
`DiagnosticReporter.reportError`. (It would have been hard to avoid
making this public API change, since the method method
`DiagnosticReporter.reportError` is already exposed publically).
To make code review easier, this CL just introduces the necessary
infrastructure to allow a diagnostic code to start supporting the new
literate API, but doesn't make the necessary modifications to any
diagnostic codes to actually support it. In a follow-up CL, I will
flip the flag `literateApiEnabled`, which will change the generated
code and cause the new literate API to be supported.
In follow-up CLs after that, I will transition the analyzer over to
reporting errors using the new API.
Change-Id: I6a6a696478fdd74803c6215c64ec68626819dd95
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445803
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change refactors the `_attachFinally` method in `_FlowAnalysisImpl`
to better align with the flow analysis specification.
Notable changes include:
- Moving `attachFinally` to the `FlowAnalysisImpl` class rather than
implicitly passing the `afterTry` model as `this`. This makes the
arguments to `attachFinally` match those in the spec.
- Extracting the handling of a single variable to a separate function,
`attachFinallyV`, to align with the spec.
- Renaming local variables to match variable names in the spec.
- Adding comments that refer to specific spec language.
- Adding parenthetical "OPTIMIZATION:" comments to document
differences between the implementation and spec that are for the
purpose of efficiency and don't affect behavior.
- Adding parenthetical "UNSPECIFIED:" comments to documented
differences between the implementaiton and spec that reflect flow
analysis features that haven't been documented yet.
- Adding comments that directly reference the `attachFinally` and
`attachFinallyV` sections of the specification.
- Reordering logic to more closely follow the structure of the
specification.
This is a pure refactoring and does not change the behavior of flow
analysis.
Change-Id: I6a6a6964bd76b87102facf4251b195e5604c7c32
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445780
Reviewed-by: Erik Ernst <eernst@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Previously, diagnostic codes were generated into libraries. This led
to a proliferation of imports of those generated libraries throughout
the analyzer and analysis server codebases, since the analysis server
didn't know that it should suggest adding imports of the correspinding
non-generated libraries instead.
I tried to fix that problem by marking each generated diagnostic code
library as deprecated, and ignoring the deprecation warning at the
site where the corresponding non-generated file imports it. But this
led to a different problem: it prevented code completion from
suggesting elements that came from the generated libraries. In my work
toward replacing the analyzer's error reporting API with a more
literate API (e.g. `reportError(errorCode.withArguments(...).at(...))`),
I've discovered that the lack of code completion makes the more
literate API much harder to use.
This CL changes the code generator so that diagnostic codes are
generated into part files. This neatly prevents unintentional imports
of the generated files without having to do any tricks with
deprecation.
A side benefit of this change is that the code generators no longer
need complex logic to determine which `import` directives to generate,
since the import directives live in the non-code-generated parent
library.
Note that in the past, the analyzer code base has heavily discouraged
the use of part files. I think they are justified in this case,
because the files are generated; if it were not for the desire to code
generate these files, we would fold them straight into the libraries
they are parts of.
Change-Id: I6a6a6964a375ee81f9580b354766bd041c83b3cd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445480
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
* No more "Fail" status in messages.status.
* Removed unused messages.
* Added examples for (almost?) all messages where it's possible.
* Add messages suite to coverage suite and update coverage.
== Additional notes
=== From the yaml spec (https://yaml.org/spec/1.2.2/)
```
2.3. Scalars
Scalar content can be written in block notation, using a literal style (indicated by “|”) where all line breaks are significant. Alternatively, they can be written with the folded style (denoted by “>”) where each line break is folded to a space unless it ends an empty or a more-indented line.
```
So we should probably use `|` and not `>` for code (especially if starting with a comment line (e.g. `// @dart=3.1`)) - and the other way around for messages.
=== These three settings
```
exampleAllowOtherCodes: true
exampleAllowMultipleReports: true
includeErrorContext: true
```
(or some combination of them)
closes https://github.com/dart-lang/sdk/issues/53634.
(one already existed, but was renamed)
Change-Id: I91a8ecee88d0600015252e289b068f60b1d1aeeb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444600
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@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>
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>
The code generation for diagnostic codes is changed so that parameter
names and their types show up in the auto-generated doc comments. When
we begin the transition to reporting diagnostics using a
statically-checked API, these comments should make it easier to figure
out what parameters to pass in.
Change-Id: I6b33ccd5b5d5a24001da2e1af2cd31cc30366e6a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444365
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This change modifies the comments in the analyzer and linter
`messages.yaml` files so that:
- Parameters are always described at the end of the comment. If the
message takes no parameters, the comment ends with the text `No
parameters.`
- Parameter descriptions are always word wrapped to 80 columns.
It also fixes the format of the comment field for
`ANALYSIS_OPTION_DEPRECATED` to remove an unintentional blank line,
and adds documentation for the second parameter of
`INVALID_EXPORT_OF_INTERNAL_ELEMENT_INDIRECTLY` (which was previously
missing).
In a follow-up CL, I will transition to a more structured format for
parameters in these `messages.yaml` files, and the `Parameters:`
sections of the comments will be automatically generated from this
structured format during code generation. Cleaning up the comments now
will ensure that the follow-up CL is clean (makes no changes to
generated files), so we'll be able to have high confidence in its
correctness.
Change-Id: I6aa1f75fa0b8e4fb9724dcba00fb7d1f06dfb355
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444205
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Phil Quitslund <pquitslund@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>
For the generated files (e.g. codes) we had gotten into a scenario
where the auto generated files was forced to be formatted as short
style, but if asking the dart formatter to format the
_fe_analyzer_shared file it would be formatted in long style because the
minimum version for _fe_analyzer_shared is now 3.7.
This CL updates the generators for the generated files to pass the
language version from the package config to the formatter.
Change-Id: I986ce1b5ff65244499ab5a277e78124f372ee46d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444300
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
This is the final CL in a series of CLs that standardizes CFE error
reporting to always use `codeFoo.withArguments(...)` when reporting
errors that take arguments and `codeFoo` when reporting errors that
don't take arguments, rather than `templateFoo.withArguments(...)`
when reporting errors that take arguments and `messageFoo` when
reporting errors that don't take arguments. This change has two
advantages:
- It lends greater consistency to the CFE codebase, by allowing the
same `code...` objects to be used both to name error codes (e.g., in
test expectations) and to report errors. This allows everything
associated with a certain error code to be found using a single
invocation of "Find References" in the editor, rather than having to
search separately for uses of the code and the message or template.
- It should hopefully make the experience of writing code that reports
errors more pleasant, since it will no longer be necessary to look
up an error to see whether it takes arguments before using it;
instead, the developer will be able to type the name of the message
`code...` declaration, and then use autocompletion to see whether
`.withArguments(...)` is required.
In this CL, the `message...` declarations that define errors are
removed, since they are no longer used (the equivalent `code...`
declarations are used instead).
Change-Id: I404e240d8d376556f147a7d80c6953bca5222f61
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443180
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This is part of a series of CLs that will standardize CFE error
reporting to always use `codeFoo.withArguments(...)` when reporting
errors that take arguments and `codeFoo` when reporting errors that
don't take arguments, rather than `templateFoo.withArguments(...)`
when reporting errors that take arguments and `messageFoo` when
reporting errors that don't take arguments. This change will have two
advantages:
- It will lend greater consistency to the CFE codebase, by allowing
the same `code...` objects to be used both to name error codes
(e.g., in test expectations) and to report errors. This will allow
everything associated with a certain error code to be found using a
single invocation of "Find References" in the editor, rather than
having to search separately for uses of the code and the message or
template.
- It should hopefully make the experience of writing code that reports
errors more pleasant, since it will no longer be necessary to look
up an error to see whether it takes arguments before using it;
instead, the developer will be able to type the name of the message
`code...` declaration, and then use autocompletion to see whether
`.withArguments(...)` is required.
In this CL, references to the `message...` declarations that define
errors are changed to the equivalent `code...` declarations. There is
no functional change, since these declarations denote the same
constant object. In a follow-up CL, the `message...` declarations will
be removed.
Tested: standard trybots
Change-Id: I44d4b3cffb768b908d4341a7851f270db6caa87b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443183
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jackson Gardner <jacksongardner@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This is part of a series of CLs that will standardize CFE error
reporting to always use `codeFoo.withArguments(...)` when reporting
errors that take arguments and `codeFoo` when reporting errors that
don't take arguments, rather than `templateFoo.withArguments(...)`
when reporting errors that take arguments and `messageFoo` when
reporting errors that don't take arguments. This change will have two
advantages:
- It will lend greater consistency to the CFE codebase, by allowing
the same `code...` objects to be used both to name error codes
(e.g., in test expectations) and to report errors. This will allow
everything associated with a certain error code to be found using a
single invocation of "Find References" in the editor, rather than
having to search separately for uses of the code and the message or
template.
- It should hopefully make the experience of writing code that reports
errors more pleasant, since it will no longer be necessary to look
up an error to see whether it takes arguments before using it;
instead, the developer will be able to type the name of the message
`code...` declaration, and then use autocompletion to see whether
`.withArguments(...)` is required.
In this CL, the code generator for error messages is changed so that
both `message...` and `code...` declarations have the type
`MessageCode` (and, indeed, refer to the same constant object). This
will allow uses of `message...` to be replaced with `code...`. In
follow-up CLs, the uses of `message...` will be changed to `code...`,
and the `message...` declarations will be removed.
Change-Id: I3ee1164678a25351c351bdb760274e84c21758ee
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443144
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This is part of a series of CLs that will standardize CFE error
reporting to always use `codeFoo.withArguments(...)` when reporting
errors that take arguments and `codeFoo` when reporting errors that
don't take arguments, rather than `templateFoo.withArguments(...)`
when reporting errors that take arguments and `messageFoo` when
reporting errors that don't take arguments. This change will have two
advantages:
- It will lend greater consistency to the CFE codebase, by allowing
the same `code...` objects to be used both to name error codes
(e.g., in test expectations) and to report errors. This will allow
everything associated with a certain error code to be found using a
single invocation of "Find References" in the editor, rather than
having to search separately for uses of the code and the message or
template.
- It should hopefully make the experience of writing code that reports
errors more pleasant, since it will no longer be necessary to look
up an error to see whether it takes arguments before using it;
instead, the developer will be able to type the name of the message
`code...` declaration, and then use autocompletion to see whether
`.withArguments(...)` is required.
In this CL, the `template...` declarations that define errors are
removed, since they are no longer used (the equivalent `code...`
declarations are used instead). In follow-up CLs, a similar set of
changes will be made to replace references to `message...`
declarations with references to the corresponding `code...`
declaration.
Change-Id: I14210cdf7b972b142b16477c46e3484eda879ef6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442733
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This is part of a series of CLs that will standardize CFE error
reporting to always use `codeFoo.withArguments(...)` when reporting
errors that take arguments and `codeFoo` when reporting errors that
don't take arguments, rather than `templateFoo.withArguments(...)`
when reporting errors that take arguments and `messageFoo` when
reporting errors that don't take arguments. This change will have two
advantages:
- It will lend greater consistency to the CFE codebase, by allowing
the same `code...` objects to be used both to name error codes
(e.g., in test expectations) and to report errors. This will allow
everything associated with a certain error code to be found using a
single invocation of "Find References" in the editor, rather than
having to search separately for uses of the code and the message or
template.
- It should hopefully make the experience of writing code that reports
errors more pleasant, since it will no longer be necessary to look
up an error to see whether it takes arguments before using it;
instead, the developer will be able to type the name of the message
`code...` declaration, and then use autocompletion to see whether
`.withArguments(...)` is required.
In this CL, the references to the `template...` declarations that
define errors are changed to the equivalent `code...`
declarations. There is no functional change, since these declarations
denote the same constant object. In follow-up CLs, the `template...`
declarations will be removed, and then a similar set of changes will
be made to replace references to `message...` declarations with
references to the corresponding `code...` declaration.
Tested: normal trybots
Change-Id: I178bd2072349088f342bc39cfc789bcb5c7ef19e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442732
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
This is part of a series of CLs that will standardize CFE error
reporting to always use `codeFoo.withArguments(...)` when reporting
errors that take arguments and `codeFoo` when reporting errors that
don't take arguments, rather than `templateFoo.withArguments(...)`
when reporting errors that take arguments and `messageFoo` when
reporting errors that don't take arguments. This change will have two
advantages:
- It will lend greater consistency to the CFE codebase, by allowing
the same `code...` objects to be used both to name error codes
(e.g., in test expectations) and to report errors. This will allow
everything associated with a certain error code to be found using a
single invocation of "Find References" in the editor, rather than
having to search separately for uses of the code and the message or
template.
- It should hopefully make the experience of writing code that reports
errors more pleasant, since it will no longer be necessary to look
up an error to see whether it takes arguments before using it;
instead, the developer will be able to type the name of the message
`code...` declaration, and then use autocompletion to see whether
`.withArguments(...)` is required.
In this CL, the code generator for error messages is changed so that
both `template...` and `code...` declarations have the static type
`Template` (and, indeed, refer to the same constant
object). Previously, `code...` declarations had the static type
`Code`, and referred to a constant object with runtime type
`Code`. This will allow uses of `template...` to be replaced with
`code...`. In follow-up CLs, the uses of `template...` will be changed
to `code...`, and the `template...` declarations will be removed.
Since the `toString` methods on `Template` and `Code` differ, I had to
make some changes to ID tests and parser tests, both of which depend
on the behavior of `toString`:
- For ID tests, I was able to preserve the existing test expectations
by modifying the `errorsToText` function (in
`pkg/front_end/lib/src/testing/id_testing_utils.dart`).
- For parser tests, I wasn't able to find an easy way to preserve the
existing test expectations, so I updated the expectations to match
the new behavior.
Change-Id: I8a461d451b06dd2f88b3e59f0cb2153711b09461
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442731
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This is part of a series of CLs that will standardize CFE error
reporting to always use `codeFoo.withArguments(...)` when reporting
errors that take arguments and `codeFoo` when reporting errors that
don't take arguments, rather than `templateFoo.withArguments(...)`
when reporting errors that take arguments and `messageFoo` when
reporting errors that don't take arguments. This change will have two
advantages:
- It will lend greater consistency to the CFE codebase, by allowing
the same `code...` objects to be used both to name error codes
(e.g., in test expectations) and to report errors. This will allow
everything associated with a certain error code to be found using a
single invocation of "Find References" in the editor, rather than
having to search separately for uses of the code and the message or
template.
- It should hopefully make the experience of writing code that reports
errors more pleasant, since it will no longer be necessary to look
up an error to see whether it takes arguments before using it;
instead, the developer will be able to type the name of the message
`code...` declaration, and then use autocompletion to see whether
`.withArguments(...)` is required.
In this CL, the `Template` class is changed so that it extends
`Code`. In follow up CLs, this will allow the `Template` objects to
serve the dual role of being either a template or an error code, in
much the same way that for errors that don't take arguments, the
`MessageCode` objects serve the dual role of being either an error
code or an error message.
Change-Id: I2e25419e47d2b0b601488b929ab5415d7039abea
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443002
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This is part of a series of CLs that will standardize CFE error
reporting to always use `codeFoo.withArguments(...)` when reporting
errors that take arguments and `codeFoo` when reporting errors that
don't take arguments, rather than `templateFoo.withArguments(...)`
when reporting errors that take arguments and `messageFoo` when
reporting errors that don't take arguments. This change will have two
advantages:
- It will lend greater consistency to the CFE codebase, by allowing
the same `code...` objects to be used both to name error codes
(e.g., in test expectations) and to report errors. This will allow
everything associated with a certain error code to be found using a
single invocation of "Find References" in the editor, rather than
having to search separately for uses of the code and the message or
template.
- It should hopefully make the experience of writing code that reports
errors more pleasant, since it will no longer be necessary to look
up an error to see whether it takes arguments before using it;
instead, the developer will be able to type the name of the message
`code...` declaration, and then use autocompletion to see whether
`.withArguments(...)` is required.
In this CL, the arguments `index`, `analyzerCodes`, and `severity` are
added to the `Template` constructor, and those arguments are supplied
by the generated code. But the arguments are not used yet. In follow
up CLs, I will change the `Template` class so that it extends `Code`
and passes these arguments along to the `Code` constructor. This in
turn will allow the `Template` objects to serve the dual role of being
either a template or an error code, in much the same way that for
errors that don't take arguments, the `MessageCode` objects serve the
dual role of being either an error code or an error message.
Change-Id: Id8d7afab0845048db96d57f68ca91259a6dbaf5e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443001
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Flow analysis didn't handle invalid type so non-null promotion would occur on declaration and initialization on erroneous code, leading to warnings about null-aware that is likely valid. For instance
f(Unresolved o) { // Error: Unresolved is unresolved
int? i = o.property;
i?.isEven; // Warning about unnecessary null-aware access
if (i != null) { // Warning about unnecessary null comparison
i.isEven;
}
}
To handle this fully we need to track invalid nullability (if that is even feasible) but for now we change the default to avoid non-null promotion in such cases.
This *does* change the kind cascading errors/warnings that we produce. For instance
f(Unresolved o) { // Error: Unresolved is unresolved
int? i = o.nonNullProperty;
i.isEven; // Error for access on int?
}
but since it probably more likely for code to *not* depend on non-null promotion, this should be less noise for the user.
Change-Id: Ia2bc3505a43b52e5151b93a7fee24e95246b4bbc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443320
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
The analyzer already has an enum with the same name, but a slightly
different declaration. (The analyzer's enum declares only severities
of `error`, `warning`, and `info`, whereas the CFE's enum also
declares severities of `context`, `ignored`, and `internalProblem`).
I'm currently embarking on an arc of work that I hope will eventually
culminate in unifying the analyzer and CFE diagnostic message
representations (and their severities) into a single set of
classes. Until that unification is complete, both representations will
have to co-exist in the `_fe_analyzer_shared` package. To reduce
confusion during that time period, I would like the classes to have
distinct names.
Since the analyzer's `Severity` enum is exposed through the analyzer
public API, analyzer clients may depend on the name. So it makes sense
to rename the CFE's `Severity` enum.
Tested: standard trybots
Change-Id: I95622950f49b1754267e441e4636e046045629bb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442102
Reviewed-by: Liam Appelbe <liama@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Previously, flow analysis represented a promotion chain as
`List<Type>?`, with `null` representing an empty promotion chain. This
was an unnecessary optimization, and it was a source of confusion when
comparing the implementation of flow analysis to the spec.
This CL changes the representation of a promotion chain to a
non-nullable `List<Type>`, and represents an empty promotion chain as
an empty list.
To reduce the risk of mistakes, I've tried to minimize the changes to
flow analysis unit tests; for the most part, they still consider an
empty promotion chain to be represented as `null`, and convert to the
new representation at the last minute. In a follow-up CL, I'll modify
the flow analysis unit tests to better follow the new representation.
Change-Id: I72afd18f9d7729f3109e3e3b5c776c5a23860985
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443540
Reviewed-by: Erik Ernst <eernst@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The analyzer already has a class with the same name, but a slightly
different purpose. (The analyzer's class represents a single message
associated with a source location, whereas the CFE's class represents
a message along with related context messages).
I'm currently embarking on an arc of work that I hope will eventually
culminate in unifying the analyzer and CFE diagnostic message
representations into a single set of classes. Until that unification
is complete, both representations will have to co-exist in the
`_fe_analyzer_shared` package. To reduce confusion during that time
period, I would like the classes to have distinct names.
Since the analyzer's `DiagnosticMessage` class is exposed through the
analyzer public API, analyzer clients may depend on the name. So it
makes sense to rename the CFE's `DiagnosticMessage` class.
Tested: standard trybots
Change-Id: I6b6948fe9da18c7b6688333fa12ffeea8e81436f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/441831
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
The following changes are made to `rebasePromotedTypes`, to make it
more accurately reflect the specification changes in
https://github.com/dart-lang/language/pull/4427.
- The order of arguments is reversed to match the order of arguments
in the spec.
- The arguments are renamed to match the argument names in the spec,
and they are changed into named arguments so that the call sites are
clearer.
- Comments are updated to reflect which parts of the algorithm come
from the spec and which parts are optimizations.
There is no functional change.
Change-Id: I0d45654ad14a6414bc1cba7c56cfc66b561c7441
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443462
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
This splits handleEndingBinaryExpression into two new listeners handleDotAccess for `.` and `?.` access and handleCascadeAccess for `..` and `?..`, both with an explicit `isNullAware` flag.
This is a step towards handling `a.b` different from `a + b` in the parser such that listeners don't have to create a value for `b` the works in both use cases; in the first is just a named operation performed on the receiver, whereas in the second case it is a full expression in its own right.
Change-Id: I04ec80401f8f2dbb8dffa86543c434e530b18cf7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442821
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
This type argument was never used. Removing it should make it easier
to make changes to the front end error message logic (and associated
code generator), which I intend to do as part of a long term plan to
make the analyzer and front end error reporting systems more
consistent, and eventually merge them.
Change-Id: I994cd2a74c5fd77df192f6a1f2dde46ba8a31883
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442723
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This moves implementation handling calls to handleEndingBinaryExpression into its own method in the subclasses. This splits the `.`, `.?`, `..` and `?..` from the real binary expressions.
This is a step towards handling `a.b` different from `a + b` in the parser such that listeners don't have to create a value for `b` the works in both use cases; in the first is just a named operation performed on the receiver, whereas in the second case it is a full expression in its own right.
Change-Id: I5291439f333971b7ff482325a15502e4d8a66b71
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442820
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This adds `LookupResult.isInvalidLookup` to the handle invalid lookup
results and uses this to avoid a lot of cascading error messages.
This is a step towards removing ProblemBuilder, AmbiguousBuilder and
reliance on `NamedBuilder.isDuplicate` in lookups.
Change-Id: Ia9d558ce55b45567607282295dd1e54e6187f9c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/441880
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
The algorithms for computing the greatest and lowest closures of a
type schema are shared between the Analyzer and the CFE. As a
work-around for the discrepancy between the behavior of the tools, the
interface of the greatest closure computation accepts the top type to
be used as one of its parameters.
Part of https://github.com/dart-lang/sdk/issues/54902
Change-Id: I378c562c39078b20c48015cdf44eb652c8d4c000
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/438660
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
This change mainly has side effects for code completion, making sure we're suggesting the right members. When the user writes something like `E e = .^` where `^` is the cursor, they're most likely intending to write a dot shorthand.
So this parser recovery change ensures that we recover as a dot shorthand node and not as a prefixed identifier (missing it's target) in the analyzer. We don't need special casing in the code completion pass to handle dot shorthands in the prefixed identifier visitor now.
The CFE should have no notable changes, but I added a few tests since it uncovered some crashing behavior. The duplicate errors have always been the case even prior to this change.
Bug: https://github.com/dart-lang/sdk/issues/59836
Change-Id: I6c4a9f2c8cc376e4dedd715af76967da471aa681
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/439140
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Paul Berry