With this change, the test runner correctly recognizes
`pkg/test_runner/test/update_errors_test.dart` as a file that's
expected _not_ to contain errors. Previously, it mistook it as a test
that was expected to contain errors, so it skipped testing it
entirely.
Change-Id: I4613215b38406b1765cb008eb33e8b076a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/491340
Auto-Submit: Paul Berry <paulberry@google.com>
Reviewed-by: Ivan Inozemtsev <iinozemtsev@google.com>
Commit-Queue: Ivan Inozemtsev <iinozemtsev@google.com>
The old formatter had a special rule that if a line comment as at the
left edge of the page, it would stay there regardless of the surrounding
indentation. So if you had:
```
class C {
m() {
// comment
}
}
```
After formatting, the comment would still be there instead of being
indented. The intent of that was to not shift over code that had been
commented out.
But all of the IDEs I tested don't actually work that way. When they
comment out code, they tend to put the `//` at the indentation of the
surrounding code. So the new formatter doesn't have this special rule
and always indents line comments following the surrounding indentation.
This is good because it also means that code generators that don't write
any leading whitespace will still get nicely formatted comments.
However, the static error test updater took advantage of this rule and
would write static error marker comments at column zero if needed to
get the carets to align with the code on the previous line and assume
that the formatter wouldn't move the comment.
This fixes the static error test updater. It always writes comments
using indentation from the previous line of code and if the caret
doesn't fit that way, it uses an explicit column marker.
Fix#57042.
Change-Id: I40fd7cd19d08dc228b6a6797e6a26965d1343d32
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/394363
Reviewed-by: Nate Bosch <nbosch@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Nate Bosch <nbosch@google.com>
Based on abandoned change #264261.
Changes to that PR: compare and show file paths, improve part parsing, include analyzer expectations from other files as well as CFE expectations, update unit tests.
Add end to end tests. The simplest case "library_failure_test.dart" passes before this PR, all others require the change in this PR to pick up expectations in other files.
Change-Id: Ia71b78a8f0dced83f603309877132f261b47c5a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345541
Reviewed-by: Erik Ernst <eernst@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Morgan :) <davidmorgan@google.com>
Error tests with explicit line/column markers, like
[error line 123, column 456]
currently require both "line" and "column".
All existing occurrences of `[error line 123, column 456]` always
refer to the most recent non-comment line *just like* the `// ^^` lines,
so omitting the line, and having the most recent non-comment line
as the *default* line, seems reasonable.
This CL makes the line optional, the new format without a "line" entry
will be just `[error column 456]`.
This allows files to change the number of lines in the code without
invalidating any later expectations.
(Or, say, remove a comment line at the top of the file!)
Omits the line entry by default when writing/updating expectations.
Change-Id: Ia0c95bf804a1c6b11c0c1621bfd481e43f8fa0c0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200429
Commit-Queue: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Currently only CFE ("Fasta") tests have their context message output
parsed. It should be easy to extend that to dart2js and DDC if that's
useful. Analyzer might be more work.
This also adds support to the test updater for inserting context
messages when updating tests. By default, that flag is off, so the
existing behavior is preserved where context messages are ignnored. If
you want them, pass "-c" when updating a test.
When validating test output, if the test file contains context messages,
then they are validated. Otherwise, any context messages in the CFE
output are ignored. This way existing tests still pass.
Change StaticError to represent a single error for a single front end.
Before, the data model collapsed errors for different front-ends at the
same location into a single StaticError object which tracked different
messages for each front end. The idea was to move towards a world where
they really are the "same" error with eventually the same message.
But this adds a lot of complexity with things like merging errors and
doesn't reflect the reality that each error from each front end is
basically its own thing. Also, critically, it makes it much harder to
attach context messages to a specific front end's error object.
This changes it so that an instance of StaticError represents a single
error for a single front end. The test file syntax is unchanged and the
updated tool behaves the same. In a static error test, multiple
expectations can still share the same "// ^^^" marker line. They are
just expanded to multiple StaticError objects at parse time.
This eliminates all of the complexity around merging and simplifying
errors.
Change-Id: Ida1736bfcde436fc2d1ce2963d91fa9cb154afa8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193281
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Before, the data model collapsed errors for different front-ends at the
same location into a single StaticError object which tracked different
messages for each front end. The idea was to move towards a world where
they really are the "same" error with eventually the same message.
But this adds a lot of complexity with things like merging errors and
doesn't reflect the reality that each error from each front end is
basically its own thing. Also, critically, it makes it much harder to
attach context messages to a specific front end's error object.
This changes it so that an instance of StaticError represents a single
error for a single front end. The test file syntax is unchanged and the
updated tool behaves the same. In a static error test, multiple
expectations can still share the same "// ^^^" marker line. They are
just expanded to multiple StaticError objects at parse time.
This eliminates all of the complexity around merging and simplifying
errors.
Change-Id: I1d55a6e885e12cc9c438f928297fc0db7dd5ce85
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193280
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
This doesn't actually run DDC to generate the web errors yet, but it
changes the CLI in anticipation of that, and adds tests to verify that
once web errors are reported that the updater handles them correctly.
Change-Id: I31264e3d468969b07f9eb60353a9b02a93bec7ea
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155102
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Previously it tested if it was at the end of file (defined as
_currentLine >= _lines.length), if it was not it would try to read the
_next_ line (_peek(1)), meaning that if _currentLine = _lines.length - 1
it would _peek(1) and try to read _lines[lines.length] and crash with a
range error. This CL fixes it by actually checking if it can safely peek
the amount it wants to.
Change-Id: I2e66c539e823f0b39bd0da39e2b2277a9e6c9501
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150624
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
This enables parsing and validation for tests that contain:
// ^^^
// [web] Some web-specific error.
This doesn't run those tests on DDC and dart2js yet, and the test
updating tool doesn't handle web tests yet. I'll do those in follow-up
patches.
Change-Id: Id06397a20a06c00e48801a16c01d5878e0264f6b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/146462
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
The CFE currently does not report the length in an error location, just
the starting position. When the static error update tool inserts an
error marker for an error reported only by the CFE, it doesn't know
what length to use.
It used to deal with this by always writing an explicit error location
like:
// [error line 1, column 9]
Those are kind of ugly and brittle, though. This changes it to treat
the error as implicitly having length 1. This way, it can just output:
// ^
When validating an error expectation against a report CFE error, the
length is ignored anyway (since the CFE doesn't report it). In order to
ensure that the parsed output of the tool matches the reported data that
produced it, the parser also ignores the length when parsing an error
expectation for a CFE-only error with length one.
Fix#37991.
Change-Id: I20e109142546b7e82a5f796a1a40613b90dc89bd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114745
Reviewed-by: Leaf Petersen <leafp@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
This rarely comes into play because most static error markers don't
have an explicit line number. (This is by design so that shifts don't
cause diff churn.) But in some cases, the line number is written. When
that happens, the number may be wrong because previous added or removed
errors can shift the output line from where the original error was
reported.
Compensate for that. Fix#37990.
Change-Id: Ie306465c11d3b6a1de5ed158ad63167b4a5d4797
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114594
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Leaf Petersen <leafp@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Konstantin Shcheglov