Skip to content
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

Deprecate GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror, boolean) #796

Merged
merged 27 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
0102cc0
remove useFlow params
AndrewShf Jul 8, 2022
6e0e1fc
a few adaptation change
Ao-senXiong Jun 26, 2024
74b72d4
Remove extra doc
Ao-senXiong Jun 26, 2024
81a7317
Set useflow as false in getDefaultAnnotations
Ao-senXiong Jun 27, 2024
1429930
Refine the reset useflow logic
Ao-senXiong Jun 27, 2024
d7c802c
Remove annotation is not needed
Ao-senXiong Jun 27, 2024
f1beeee
Use protected for useflow
Ao-senXiong Jun 27, 2024
23c0f13
Doc refinement and changelog entry
Ao-senXiong Jun 27, 2024
fe1a616
Changelog rewording from code review
Ao-senXiong Jun 28, 2024
76b2cda
Add miss right parentheses
Ao-senXiong Jun 28, 2024
8742cfb
Use `this` before calling the field
Ao-senXiong Jun 28, 2024
d27f892
Deprecate method instead removing
Ao-senXiong Jun 28, 2024
e2a60dd
Remove duplicate javadoc
Ao-senXiong Jul 3, 2024
8edaa67
Merge branch 'master' into useflow-cleanup
Ao-senXiong Jul 3, 2024
9d2e296
Address comment from discussion
Ao-senXiong Jul 4, 2024
d0f8dba
Javadoc improvement
Ao-senXiong Jul 4, 2024
f452084
Apply suggestions from code review
Ao-senXiong Jul 7, 2024
a08a209
make `addComputedTypeAnnotationsWithoutFlow` protected and final
Ao-senXiong Jul 7, 2024
d049e92
Add @see tap to useFlow field
Ao-senXiong Jul 7, 2024
1f477b1
Apply suggestions from code review
Ao-senXiong Jul 8, 2024
459692f
Apply spotless
Ao-senXiong Jul 8, 2024
18c3bfa
Update changelog
Ao-senXiong Jul 8, 2024
e93a6f4
Add description in the manual
Ao-senXiong Jul 8, 2024
0dd1232
Fix manual issue
Ao-senXiong Jul 8, 2024
0746ecf
Update docs/CHANGELOG.md
Ao-senXiong Jul 9, 2024
7586991
Improve Javadoc
Ao-senXiong Jul 9, 2024
16082e3
Update javadoc
wmdietl Jul 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ public void addComputedTypeAnnotations(Element element, AnnotatedTypeMirror type

/** Handles cases 1, 2, and 3. */
@Override
public void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type, boolean iUseFlow) {
super.addComputedTypeAnnotations(tree, type, iUseFlow);
public void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type) {
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
super.addComputedTypeAnnotations(tree, type);
// If dataflow shouldn't be used to compute this type, then do not use the result from
// the Value Checker, because dataflow is used to compute that type. (Without this,
// "int i = 1; --i;" fails.)
Expand All @@ -203,7 +203,7 @@ public void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type, bool
// checker's type factory is parsing.
&& !ajavaTypes.isParsing()
&& TreeUtils.isExpressionTree(tree)
&& (iUseFlow || tree instanceof LiteralTree)) {
&& (this.useFlow || tree instanceof LiteralTree)) {
AnnotatedTypeMirror valueType = getValueAnnotatedTypeFactory().getAnnotatedType(tree);
addLowerBoundTypeFromValueType(valueType, type);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,12 @@ public void addComputedTypeAnnotations(Element element, AnnotatedTypeMirror type
}

@Override
public void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type, boolean iUseFlow) {
super.addComputedTypeAnnotations(tree, type, iUseFlow);
public void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type) {
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
super.addComputedTypeAnnotations(tree, type);
// If dataflow shouldn't be used to compute this type, then do not use the result from
// the Value Checker, because dataflow is used to compute that type. (Without this,
// "int i = 1; --i;" fails.)
if (iUseFlow
if (useFlow
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
&& tree != null
&& !ajavaTypes.isParsing()
&& TreeUtils.isExpressionTree(tree)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -699,13 +699,13 @@ public void addComputedTypeAnnotations(Element elt, AnnotatedTypeMirror type) {
}

@Override
public void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type, boolean useFlow) {
public void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type) {
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
if (tree.getKind() == Tree.Kind.VARIABLE) {
translateJcipAndJavaxAnnotations(
TreeUtils.elementFromDeclaration((VariableTree) tree), type);
}

super.addComputedTypeAnnotations(tree, type, useFlow);
super.addComputedTypeAnnotations(tree, type);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ public SignednessAnnotatedTypeFactory(BaseTypeChecker checker) {
}

@Override
protected void addComputedTypeAnnotations(
Tree tree, AnnotatedTypeMirror type, boolean iUseFlow) {
protected void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type) {
Tree.Kind treeKind = tree.getKind();
if (treeKind == Tree.Kind.INT_LITERAL) {
int literalValue = (int) ((LiteralTree) tree).getValue();
Expand All @@ -139,8 +138,7 @@ protected void addComputedTypeAnnotations(
} else if (!isComputingAnnotatedTypeMirrorOfLhs()) {
addSignedPositiveAnnotation(tree, type);
}

super.addComputedTypeAnnotations(tree, type, iUseFlow);
super.addComputedTypeAnnotations(tree, type);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ Version 3.42.0-eisop4 (April ?, 2024)

**Implementation details:**

Removed the `GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror, boolean)` overload. Instead, set `GenericAnnotatedTypeFactory#useFlow` appropriately and call `GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror`.
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved

Improvements in `framework-test` to more consistently handle tests that do not use
`-Anomsgtext`.

Expand Down
3 changes: 0 additions & 3 deletions docs/manual/creating-a-checker.tex
Original file line number Diff line number Diff line change
Expand Up @@ -1356,9 +1356,6 @@
Create a subclass of \refclass{framework/type}{AnnotatedTypeFactory} and
override two \<addComputedTypeAnnotations> methods:
\refmethodanchortext{framework/type}{AnnotatedTypeFactory}{addComputedTypeAnnotations}{(com.sun.source.tree.Tree,org.checkerframework.framework.type.AnnotatedTypeMirror)}{addComputedTypeAnnotations(Tree,AnnotatedTypeMirror)}
(or
\refmethodanchortext{framework/type}{GenericAnnotatedTypeFactory}{addComputedTypeAnnotations}{(com.sun.source.tree.Tree,org.checkerframework.framework.type.AnnotatedTypeMirror,boolean)}{addComputedTypeAnnotations(Tree,AnnotatedTypeMirror,boolean)}
if extending \code{GenericAnnotatedTypeFactory})
and
\refmethodanchortext{framework/type}{AnnotatedTypeFactory}{addComputedTypeAnnotations}{(javax.lang.model.element.Element,org.checkerframework.framework.type.AnnotatedTypeMirror)}{addComputedTypeAnnotations(Element,AnnotatedTypeMirror)}.
The methods can make arbitrary changes to the annotations on a type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1901,10 +1901,6 @@ private AnnotatedTypeMirror fromExpression(ExpressionTree tree) {
* <p>Subclasses that override this method should also override {@link
* #addComputedTypeAnnotations(Element, AnnotatedTypeMirror)}.
*
* <p>In classes that extend {@link GenericAnnotatedTypeFactory}, override {@link
* GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror, boolean)}
* instead of this method.
*
* @param tree an AST node
* @param type the type obtained from {@code tree}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ public abstract class GenericAnnotatedTypeFactory<
*
* @see #getAnnotatedTypeLhs(Tree)
*/
private boolean useFlow;
protected boolean useFlow;
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved

/** Is this type factory configured to use flow-sensitive type refinement? */
private final boolean everUseFlow;
Expand Down Expand Up @@ -1981,17 +1981,6 @@ public void addDefaultAnnotations(AnnotatedTypeMirror type) {
defaults.annotate((Element) null, type);
}

/**
* This method is final; override {@link #addComputedTypeAnnotations(Tree, AnnotatedTypeMirror,
* boolean)} instead.
*
* <p>{@inheritDoc}
*/
@Override
protected final void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type) {
addComputedTypeAnnotations(tree, type, this.useFlow);
}

/**
* Removes all primary annotations on a copy of the type and calculates the default annotations
* that apply to the copied type, without type refinements.
Expand All @@ -2003,7 +1992,10 @@ protected final void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror t
public AnnotatedTypeMirror getDefaultAnnotations(Tree tree, AnnotatedTypeMirror type) {
AnnotatedTypeMirror copy = type.deepCopy();
copy.removeAnnotations(type.getAnnotations());
addComputedTypeAnnotations(tree, copy, false);
boolean oldUseflow = useFlow;
useFlow = false;
addComputedTypeAnnotations(tree, copy);
useFlow = oldUseflow;
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
return copy;
}

Expand All @@ -2013,10 +2005,9 @@ public AnnotatedTypeMirror getDefaultAnnotations(Tree tree, AnnotatedTypeMirror
*
* @param tree an AST node
* @param type the type obtained from tree
* @param iUseFlow whether to use information from dataflow analysis
*/
protected void addComputedTypeAnnotations(
Tree tree, AnnotatedTypeMirror type, boolean iUseFlow) {
@Override
protected void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An @see to addComputedTypeAnnotationsWithoutFlow from here would be good, to connect them in both directions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated with inheritance doc and @see tag.

if (this.getRoot() == null && ajavaTypes.isParsing()) {
return;
}
Expand All @@ -2038,7 +2029,7 @@ protected void addComputedTypeAnnotations(
}
log(
"%s GATF.addComputedTypeAnnotations#1(%s, %s, %s)%n",
thisClass, treeString, type, iUseFlow);
thisClass, treeString, type, this.useFlow);
if (!TreeUtils.isExpressionTree(tree)) {
// Don't apply defaults to expressions. Their types may be computed from subexpressions
// in treeAnnotator.
Expand All @@ -2063,7 +2054,7 @@ protected void addComputedTypeAnnotations(
defaults.annotate(tree, type);
log("%s GATF.addComputedTypeAnnotations#7(%s, %s)%n", thisClass, treeString, type);

if (iUseFlow) {
if (this.useFlow) {
Value inferred = getInferredValueFor(tree);
if (inferred != null) {
applyInferredAnnotations(type, inferred);
Expand All @@ -2074,7 +2065,7 @@ protected void addComputedTypeAnnotations(
}
log(
"%s GATF.addComputedTypeAnnotations#9(%s, %s, %s) done%n",
thisClass, treeString, type, iUseFlow);
thisClass, treeString, type, this.useFlow);
}

/**
Expand Down Expand Up @@ -2745,7 +2736,10 @@ public AnnotatedTypeMirror getDefaultValueAnnotatedType(TypeMirror typeMirror) {
TypeMirror defaultValueTM = TreeUtils.typeOf(defaultValueTree);
AnnotatedTypeMirror defaultValueATM =
AnnotatedTypeMirror.createType(defaultValueTM, this, false);
addComputedTypeAnnotations(defaultValueTree, defaultValueATM, false);
boolean oldUseflow = useFlow;
useFlow = false;
addComputedTypeAnnotations(defaultValueTree, defaultValueATM);
useFlow = oldUseflow;
return defaultValueATM;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ protected Set<Class<? extends Annotation>> createSupportedTypeQualifiers() {
}

@Override
protected void addComputedTypeAnnotations(
Tree tree, AnnotatedTypeMirror type, boolean iUseFlow) {
super.addComputedTypeAnnotations(tree, type, iUseFlow);
protected void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type) {
super.addComputedTypeAnnotations(tree, type);
if (tree.getKind() == Tree.Kind.VARIABLE
&& ((VariableTree) tree).getName().toString().contains("addH1S2")) {
type.replaceAnnotation(H1S2);
Expand Down
Loading