Skip to content

Faulty code: Make OCUndeclaredVariableWarning a little less special #13186

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

Merged

Conversation

privat
Copy link
Contributor

@privat privat commented Mar 29, 2023

This PR does some cleanup and reorganization.

In #13165 it was explicitly noted that OCASTSemanticAnalyzer had to deal with OCUndeclaredVariableWarning signaling in non-faulty-mode.
This is bad because:

  1. the semantic analyzer should not have to signal exceptions, but only annotate the AST.
  2. OCUndeclaredVariableWarning is signaled too early, and possibly before more important errors.

So this PR remove this responsibility on the semantic analyzer by making the Undeclared thing compatible with the RBNotice and CodeError infrastructure: it just annotates the AST; then possible OCUndeclaredVariableWarning are signaled along with possible CodeError.
However, it does it transparently for existing clients, so preserving the current "half-warning half-error" semantic. (See #13006 )

Here is a summary on the changes:

  • new special notice OCUndeclaredVariableNotice
  • OCASTSemanticAnalyzer does not check faulty, nor signal exceptions on undeclared variables
  • on faulty check, this new class OCUndeclaredVariableNotice does not signal CodeError but OCUndeclaredVariableWarning
  • OCUndeclaredVariableWarning is made compatible with the API of CodeError, so catcher that care can be agnostic.
    Currently, it cannot be a subclass of CodeError because doing so breaks clients and tests that catch (or expect not to catch) CodeError exceptions.

As a side effect, OCSemanticWarning that is no more used is removed.

@privat privat changed the base branch from Pharo12 to Pharo11 March 29, 2023 23:13
@privat privat changed the base branch from Pharo11 to Pharo12 March 29, 2023 23:13
@MarcusDenker MarcusDenker merged commit 86968c5 into pharo-project:Pharo12 Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants