commit df707ac85e9a81b73c80638b759aede6be80ebbb
author Sam Rawlins <srawlins@google.com> Fri Apr 05 00:17:58 2024 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Apr 05 00:17:58 2024 +0000
tree 3aa8307d71503e82bc4ae7d56712c6272c8848c3
parent 01c4d15a959d0eda29d9186f4957be65f343cd55

DAS: Refactor the logic determining the location of a new ignore-comment

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>