I renamed a few classes, then sorted them, so the diff is big.
Broadly, here is a summary of the code before: two "ignore diagnostic"
correction producers were in place, and the one that inserts a
"ignore_for_file" comment (IgnoreDiagnosticInFile) uses a
`CorrectionUtils.getInsertionLocationIgnoreForFile` function to get an
InsertionLocation object, then passes that to `_computeEdit()`, a
method defined in a parent class. `_computeEdit()` takes that
InsertionLocation object, and uses LineInfo, to determine whether we
need to _append_ to an existing ignore-comment (in which case all
other InsertionLocation information is discarded), or write a full
comment, and use the InsertionLocation prefix and suffix.
To me this was a pretty complicated set-up: IgnoreDiagnosticInFile
calls out to a function defined in a separate library to do "the first
half" of the location calculation. Then the `_computeEdit()` function
has to finish the job, which might involve ignoring computations done
in the first half.
I've changed it to instead be the following design: Each class,
IgnoreDiagnosticInFile and IgnoreDiagnosticOnLine, wholly implements
`compute()` (removing the parent `_computeEdit()` method). The former
class has the much bigger task of finding an appropriate position near
the top of the file in which it can insert a comment. Instead of
tracking a lot of variables along the way, this code quickly calls out
to `insertAt()` as soon as it knows where it is inserting, whether it is
appending, and whether we need a newline before the comment, or after.
So everything about the offset, possible prefix, suffix, and decision
to-append is done in one place. The latter class,
IgnoreDiagnosticOnLine, has a much simpler `compute()`, the most
complex part is determining whether to append to an existing comment.
There is not much duplicated. The `insertAt()` method is defined in
the parent class; it is nice to have a method that is agnostic to the
ignore comment being inserted; it just calculates an indent, and
writes the comment.
Summary of changes:
* AbstractIgnoreDiagnostic, the parent of the two mentioned classes,
and IgnoreDiagnosticInAnalysisOptionsFile, is actually a base class,
shouldn't be public, and it's `_computeEdit()` was not relevant for
the 3rd subclass. So I rename it to _BaseIgnoreDiagnostic, and add a
new small class in between: _DartIgnoreDiagnostic. The new class
defines `insertAt()` (only relevant for Dart files), and requires
that subclasses define their `ignorePrefix`.
* `_isCodeUnignorable` is changed from a method to a getter.
* `DartEditBuilderImpl._linePrefix` is moved to an extension on ResolvedUnitResult, along with some helpers. It is moved to `src/` so
not technically part of public API. It should be moved to
server_plugin soon.
* `getInsertionLocationIgnoreForFile` and `getLinePrefix` are removed.
Change-Id: I4757ea2d8a3b43eeec0c8895c5c1fb3edc3ca8a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361006
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Sam Rawlins
df707ac85e