search

eisop / checker-framework

Pluggable type-checking for Java
https://eisop.github.io/
Other
15 stars 16 forks source link

Deprecated `GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror, boolean)` #796

Open Ao-senXiong opened 1 week ago

Ao-senXiong commented 1 week ago

Cherry-pick from https://github.com/opprop/checker-framework/pull/215, we can close that PR after merge this one.

Ao-senXiong commented 1 week ago

I still like this change, I think removing the extra parameter is good. Can you add a changelog and also see whether the method is mentioned in the manual?

Added a changelog entry and remove document no longer useful in the manual.

wmdietl commented 1 week ago

Instead of removing the method we could deprecate it and implement it by setting the corresponding field, like we do in a few places. That way we wouldn't directly break anyone.

Ao-senXiong commented 1 week ago

Instead of removing the method we could deprecate it and implement it by setting the corresponding field, like we do in a few places. That way we wouldn't directly break anyone.

Sure, already added back GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror, boolean) in genericATF by calling the GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror) inside it. I think we will have less code duplication in this way. Also updated the changelog.

Ao-senXiong commented 3 days ago

You say I think we will have less code duplication in this way. What do you mean? How does the deprecation avoid duplication?

I have adapted this method addComputedTypeAnnotations( Tree tree, AnnotatedTypeMirror type, boolean iUseFlow) to call addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type) in its body instead of have two methods with very similar method body.

Ao-senXiong commented 1 day ago

Hi @wmdietl, could you review the PR and see how do you like the code change now?