Skip to content

Rename _fe_analyzer_shared's ErrorCode #60635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
srawlins opened this issue Apr 29, 2025 · 3 comments
Open

Rename _fe_analyzer_shared's ErrorCode #60635

srawlins opened this issue Apr 29, 2025 · 3 comments
Assignees
Labels
area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable

Comments

@srawlins
Copy link
Member

The different "types" of diagnostics that the analyzer (and CFE?) can report exist as classes (like LintCode, ParserErrorCode, TodoCode, StaticWarningCode, CompileTimeErrorCode, WarningCode, ScannerErrorCode, AnalysisOptionsErrorCode, AnalysisOptionsWarningCode, etc), which all have a common super-class: ErrorCode.

This name is potentially confusing, as we can say, for example "using a @visibleForTesting element outside of a test does not yield an error; it yields a warning." But warning is a subclass of error? For years this confusion would only hamper the developers of the analyzer (and CFE?). But the new analyzer plugin system uses LintCode and WarningCode, and we need a better name for the shared super-class, for function signatures, etc.

I propose DiagnosticCode. It's longer, more syllables, but it is not a name that will be used often.

We can do a gentle incremental migration, and there is no rush to delete ErrorCode:

  1. Add a type alias, typedef DiagnosticCode = ErrorCode.
  2. Change function signatures in API that will become public for analyzer plugins.

Then, as needed, as time permits:

  1. Migrate internal usage to DiagnosticCode.
  2. Deprecate ErrorCode.
  3. In a major version release, without any rush, rename ErrorCode to DiagnosticCode and remove the type alias.
@srawlins srawlins added area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable labels Apr 29, 2025
@johnniwinther
Copy link
Member

@srawlins would you be willing to take on this task?

@srawlins
Copy link
Member Author

Absolutely I would! 😁

copybara-service bot pushed a commit that referenced this issue Apr 29, 2025
Work towards #60635

In this CL I only introduce the typedefs, update comments, and export
the typedefs. I don't change any references. Next we should update
internal references and maybe separately, any generated references.

Change-Id: I1c6d16580533b9283934261f56a6e5237e59109e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425343
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
@srawlins
Copy link
Member Author

The next big one to rename might be AnalysisError, in the analyzer package.

This is sort of the "instances" of DiagnosticCodes. Each AnalysisError represents a reported diagnostic. In fact, AnalysisError extends a class, Diagnostic, also defined in the analyzer package. This is curious...

Diagnostic has exactly one direct subclass (AnalysisError has a few more for tests). Diagnostic 7 times in all of analyzer. Maybe someone started attempting this migration already?

My proposal for AnalysisError: scoop all of its implementation into Diagnostic:

  1. Remove unused members (correction?). (Well, deprecate; it's public API.)
  2. Move used members to Diagnostic.
  3. Make its subclasses sublcass Diagnostic instead.
  4. Deprecate the whole class.
  5. Remove the whole class.

copybara-service bot pushed a commit that referenced this issue Apr 30, 2025
Work towards #60635

Change-Id: I76c3f7b3ed890b808f3831f2b7776bd1988a223d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425407
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 30, 2025
Work towards #60635

Change-Id: Ia021bc5a80f1e53407393454ab058f6b7c8a633a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425460
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 30, 2025
Work towards #60635

Change-Id: Id80912299c8bd9caafb9f6a9521168c74911f90d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425581
Reviewed-by: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 30, 2025
Work towards #60635

Change-Id: I1d36abb9f98ae875697ac54b9e493ffdb8666f75
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425580
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 30, 2025
…elds

Work towards #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 <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 1, 2025
Work towards #60635

Change-Id: I3f53e1c7db21047665e1a286673d624c2d48c834
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425684
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 1, 2025
Work towards #60635

Diagnostic has exactly one direct subclass, AnalysisError
(AnalysisError has a few more for tests). Diagnostic was previously
referenced only 7 times in all of analyzer.

This change is essentially a no-op.

Change-Id: I043970d96adcd7be4bb6a24d4a85b3f74309b7d6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425682
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 2, 2025
Work towards #60635

Change-Id: I942e690ab2946c564c10df228d17bd44de4ed375
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425406
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 4, 2025
Work towards #60635

DiagnosticSeverity is the new name for ErrorSeverity.

Change-Id: I9d1040f23d4fe8affb234ff62527133e439adf50
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426341
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 4, 2025
Work towards #60635

Change-Id: Ifce5f4919f5bb8e12ac73994263d1a9fe745e0e9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426402
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 5, 2025
Work towards #60635

Change-Id: I0107a65580bc41d769c2a384319225b256df2fbe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426420
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 5, 2025
Work towards #60635

DiagnosticSeverity is the new name for ErrorSeverity.

Change-Id: I23a0e3d487934cea2f44a673215bc22faf34ee52
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426000
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 5, 2025
Work towards #60635

Change-Id: Icf62987d6de7cafb6f6e92076a5fcc2bd48772c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426522
Auto-Submit: Samuel Rawlins <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 5, 2025
…Error

Work towards #60635

Change-Id: Icf4186098ed04f15395aaf06c18772f8fd6ec4b7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426560
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 5, 2025
Work towards #60635

Change-Id: Ic7f84584d4185e1ab7e8741052ec6694487c8c08
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426600
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 5, 2025
Work towards #60635

Change-Id: I225bcbe2c0033872b0349258f7f2135aab6a8d20
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426561
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 6, 2025
Work towards #60635

Change-Id: I0cfc0bff4e5d10b7cb373f77de55fffc13b8cf76
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426641
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 8, 2025
Work towards #60635

Change-Id: If9bbc96beed95129b099588c5bd9728afda8e392
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426902
Reviewed-by: Nate Bosch <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Bob Nystrom <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable
Projects
None yet
Development

No branches or pull requests

2 participants