-
Notifications
You must be signed in to change notification settings - Fork 142
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
Eclipse reports Type mismatch for @Deprecated class #1446
Eclipse reports Type mismatch for @Deprecated class #1446
Conversation
+ more systematic approach to set/reset ProblemHandler.referenceContext
/** | ||
* General strategy: | ||
* <ul> | ||
* <li>clients are responsible for assigning {@link #referenceContext} prior to invoking any | ||
* reporting method (usually, by obtaining an instance using {@link Scope#problemReporter()}). | ||
* <li>individual methods should ensure that {@link #referenceContext} is reset to {@code null} afterwards. This may | ||
* happen in two ways: | ||
* <ul><li>by calling any of the {@code handle(..)} methods on all paths, or failing that | ||
* <li>wrap their body in {@code try(this) { ... }}. | ||
* </ul></ul> | ||
*/ | ||
@Override | ||
public void close() { | ||
this.referenceContext = null; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@srikanth-sankaran @jarthana One more change that tries to make an implicit strategy more explicit. Do you want to comment on changes outlined in the above javadoc?
I briefly thought of actually pulling all this into a new method in Scope
, to replace all those scope.problemReporter().reportXYZ(foo,bar)
calls with smth like this:
scope.withContext(pr -> pr.reportXYZ(foo,bar));
This would encapsulate that strategy even better, but it would require a million of edits all over the place ;-P -- so it may be no more than a theoratical exercise ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should mention that this change was motivated by the observation that (a) my new method scheduleProblemForContext()
was implemented incompletely, and (b) nobody noticed, because we accidentally benefit from other methods being sloppy wrt ProblemReporter#referenceContext
, notably all those calls to problemReporter().validateRestrictedKeywords()
, which normally never reset the context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@srikanth-sankaran @jarthana One more change that tries to make an implicit strategy more explicit. Do you want to comment on changes outlined in the above javadoc?
Of course I will accept your comments any time, despite having merged this PR.
Eclipse reports Type mismatch for @deprecated class fixes eclipse-jdt#1412 + Defer problem reporting if it would require ahead-of-time resolving of annotations (tasks stored in CompilationResult). Materialize such problems just at the beginning of CompilationUnitDeclaration.finalizeProblems(). + more systematic approach to set/reset ProblemHandler.referenceContext
fixes #1412
What it does
Defer problem reporting if it would require ahead-of-time resolving of annotations (tasks stored in
CompilationResult
).Materialize such problems just at the beginning of
CompilationUnitDeclaration.finalizeProblems()
.How to test
JUnit included