-
Notifications
You must be signed in to change notification settings - Fork 13
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
Update to EISOP 3.39-eisop1 from 3.34-eisop1 #439
Update to EISOP 3.39-eisop1 from 3.34-eisop1 #439
Conversation
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.
Thanks for performing this large merge! I appreciate that tracking down all these changes took time.
Tree varTree, | ||
ExpressionTree valueExp, | ||
@CompilerMessageKey String errorKey, | ||
Object... extraArgs) { | ||
if (!validateTypeOf(varTree)) { | ||
return; | ||
return true; |
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.
Why true
if the tree isn't valid?
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.
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.
That seems wrong over there, too, doesn't it? Let's clean this up in separate PRs.
@@ -583,7 +594,7 @@ protected void commonAssignmentCheck( | |||
mono.getCanonicalName(), | |||
mono.getCanonicalName(), | |||
valueType.toString()); | |||
return; | |||
return true; |
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.
The true
is again confusing. Can you add a comment that clarifies?
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.
Oh, sorry, this one should be return false because of the check fails. And the code here is outdate with super method and should be updated in a seperate PR. @wmdietl I think I will update the copied code after this update.
Co-authored-by: Werner Dietl <wdietl@gmail.com>
@@ -159,7 +159,7 @@ public void matchAndReplacePrimary(final AnnotatedTypeMirror typeUse, final Anno | |||
} | |||
|
|||
@Override | |||
protected String defaultErrorMessage(org.checkerframework.framework.type.AnnotatedTypeMirror type1, org.checkerframework.framework.type.AnnotatedTypeMirror type2, Void aVoid) { | |||
public String defaultErrorMessage(org.checkerframework.framework.type.AnnotatedTypeMirror type1, org.checkerframework.framework.type.AnnotatedTypeMirror type2, Void aVoid) { |
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.
In a separate PR, look why this uses the fully-qualified name. The method before uses just ATM.
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.
Thanks!
If this get merged, we can close #413.
PICO's change will be pull in gradually after this change.