Skip to content

Commit c115afa

Browse files
benzonicobenzonico
committed
SONARJAVA-1416 cleanup after rebase
1 parent 3a2ae77 commit c115afa

3 files changed

Lines changed: 109 additions & 135 deletions

File tree

‎java-squid/src/main/java/org/sonar/java/se/ExplodedGraphWalker.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -523,15 +523,13 @@ private void executeIdentifier(IdentifierTree tree) {
523523
}
524524

525525
private void executeMemberSelect(MemberSelectExpressionTree mse) {
526-
if (!mse.parent().is(Tree.Kind.METHOD_INVOCATION)) {
527-
if (!"class".equals(mse.identifier().name())) {
526+
if (!"class".equals(mse.identifier().name())) {
528527

529-
ProgramState.Pop unstackMSE = programState.unstackValue(1);
530-
programState = unstackMSE.state;
531-
}
532-
SymbolicValue mseValue = constraintManager.createSymbolicValue(mse);
533-
programState = programState.stackValue(mseValue);
528+
ProgramState.Pop unstackMSE = programState.unstackValue(1);
529+
programState = unstackMSE.state;
534530
}
531+
SymbolicValue mseValue = constraintManager.createSymbolicValue(mse);
532+
programState = programState.stackValue(mseValue);
535533
}
536534

537535
public void clearStack(Tree tree) {

‎java-squid/src/main/java/org/sonar/java/se/ProgramState.java‎

Lines changed: 4 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.ArrayList;
3434
import java.util.Collections;
3535
import java.util.Deque;
36+
import java.util.HashMap;
3637
import java.util.HashSet;
3738
import java.util.Iterator;
3839
import java.util.LinkedList;
@@ -43,17 +44,6 @@
4344

4445
public class ProgramState {
4546

46-
public static class ConstrainedValue {
47-
48-
public final SymbolicValue value;
49-
public final ObjectConstraint constraint;
50-
51-
ConstrainedValue(SymbolicValue value, ObjectConstraint constraint) {
52-
this.value = value;
53-
this.constraint = constraint;
54-
}
55-
}
56-
5747
public static class Pop {
5848

5949
public final ProgramState state;
@@ -341,31 +331,15 @@ public SymbolicValue getValue(Symbol symbol) {
341331
return values.get(symbol);
342332
}
343333

344-
public List<ObjectConstraint> getConstraints(final Object state) {
345-
final List<ObjectConstraint> result = new ArrayList<>();
346-
constraints.forEach(new PMap.Consumer<SymbolicValue, Object>() {
347-
@Override
348-
public void accept(SymbolicValue value, Object valueConstraint) {
349-
if (valueConstraint instanceof ObjectConstraint) {
350-
ObjectConstraint constraint = (ObjectConstraint) valueConstraint;
351-
if (constraint.hasStatus(state)) {
352-
result.add(constraint);
353-
}
354-
}
355-
}
356-
});
357-
return result;
358-
}
359-
360-
public List<ConstrainedValue> getValuesWithConstraints(final Object state) {
361-
final List<ConstrainedValue> result = new ArrayList<>();
334+
public Map<SymbolicValue, ObjectConstraint> getValuesWithConstraints(final Object state) {
335+
final Map<SymbolicValue, ObjectConstraint> result = new HashMap<>();
362336
constraints.forEach(new PMap.Consumer<SymbolicValue, Object>() {
363337
@Override
364338
public void accept(SymbolicValue value, Object valueConstraint) {
365339
if (valueConstraint instanceof ObjectConstraint) {
366340
ObjectConstraint constraint = (ObjectConstraint) valueConstraint;
367341
if (constraint.hasStatus(state)) {
368-
result.add(new ConstrainedValue(value, constraint));
342+
result.put(value, constraint);
369343
}
370344
}
371345
}

‎java-squid/src/main/java/org/sonar/java/se/checks/UnclosedResourcesCheck.java‎

Lines changed: 100 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import org.sonar.java.se.ConstraintManager;
2929
import org.sonar.java.se.ObjectConstraint;
3030
import org.sonar.java.se.ProgramState;
31-
import org.sonar.java.se.ProgramState.ConstrainedValue;
3231
import org.sonar.java.se.SymbolicValue;
3332
import org.sonar.java.se.SymbolicValueFactory;
3433
import org.sonar.plugins.java.api.JavaFileScanner;
@@ -67,11 +66,13 @@
6766
public class UnclosedResourcesCheck extends SECheck implements JavaFileScanner {
6867

6968
private enum Status {
70-
OPENED, CLOSED;
69+
OPENED, CLOSED
7170
}
7271

7372
private static final String JAVA_IO_AUTO_CLOSEABLE = "java.lang.AutoCloseable";
7473
private static final String JAVA_IO_CLOSEABLE = "java.io.Closeable";
74+
private static final String JAVA_SQL_STATEMENT = "java.sql.Statement";
75+
private static final String[] JDBC_RESOURCE_CREATIONS = {"java.sql.Connection", JAVA_SQL_STATEMENT};
7576
private static final String[] IGNORED_CLOSEABLE_SUBTYPES = {
7677
"java.io.ByteArrayOutputStream",
7778
"java.io.ByteArrayInputStream",
@@ -81,10 +82,47 @@ private enum Status {
8182
"java.io.StringWriter",
8283
"com.sun.org.apache.xml.internal.security.utils.UnsyncByteArrayOutputStream"
8384
};
84-
private static final String JAVA_SQL_STATEMENT = "java.sql.Statement";
85-
private static final String[] JDBC_RESOURCE_CREATIONS = {"java.sql.Connection", JAVA_SQL_STATEMENT};
8685

87-
static boolean needsClosing(Type type) {
86+
@Override
87+
public void scanFile(JavaFileScannerContext context) {
88+
Multimap<Tree, String> issues = ((DefaultJavaFileScannerContext) context).getSEIssues(UnclosedResourcesCheck.class);
89+
for (Map.Entry<Tree, String> issue : issues.entries()) {
90+
context.reportIssue(this, issue.getKey(), issue.getValue());
91+
}
92+
}
93+
94+
@Override
95+
public ProgramState checkPreStatement(CheckerContext context, Tree syntaxNode) {
96+
final PreStatementVisitor visitor = new PreStatementVisitor(context);
97+
syntaxNode.accept(visitor);
98+
return visitor.programState;
99+
}
100+
101+
@Override
102+
public ProgramState checkPostStatement(CheckerContext context, Tree syntaxNode) {
103+
final PostStatementVisitor visitor = new PostStatementVisitor(context);
104+
syntaxNode.accept(visitor);
105+
return visitor.programState;
106+
}
107+
108+
@Override
109+
public void checkEndOfExecutionPath(CheckerContext context, ConstraintManager constraintManager) {
110+
final List<ObjectConstraint> constraints = context.getState().getFieldConstraints(Status.OPENED);
111+
for (ObjectConstraint constraint : constraints) {
112+
Tree syntaxNode = constraint.syntaxNode();
113+
String name = null;
114+
if (syntaxNode.is(Tree.Kind.NEW_CLASS)) {
115+
name = ((NewClassTree) syntaxNode).identifier().symbolType().name();
116+
} else if (syntaxNode.is(Tree.Kind.METHOD_INVOCATION)) {
117+
name = ((MethodInvocationTree) syntaxNode).symbolType().name();
118+
}
119+
if(name != null) {
120+
context.reportIssue(syntaxNode, this, "Close this \"" + name + "\".");
121+
}
122+
}
123+
}
124+
125+
private static boolean needsClosing(Type type) {
88126
for (String ignoredTypes : IGNORED_CLOSEABLE_SUBTYPES) {
89127
if (type.is(ignoredTypes)) {
90128
return false;
@@ -93,32 +131,28 @@ static boolean needsClosing(Type type) {
93131
return isCloseable(type);
94132
}
95133

96-
static boolean isCloseable(Type type) {
134+
private static boolean isCloseable(Type type) {
97135
return type.isSubtypeOf(JAVA_IO_AUTO_CLOSEABLE) || type.isSubtypeOf(JAVA_IO_CLOSEABLE);
98136
}
99137

100-
static boolean isOpeningResource(NewClassTree syntaxNode) {
138+
private static boolean isOpeningResource(NewClassTree syntaxNode) {
101139
if (isWithinTryHeader(syntaxNode)) {
102140
return false;
103141
}
104-
final Type type = syntaxNode.symbolType();
105-
return needsClosing(type);
142+
return needsClosing(syntaxNode.symbolType());
106143
}
107144

108-
static boolean isWithinTryHeader(Tree syntaxNode) {
145+
private static boolean isWithinTryHeader(Tree syntaxNode) {
109146
final Tree parent = syntaxNode.parent();
110147
if (parent.is(Tree.Kind.VARIABLE)) {
111148
return isTryStatementResource((VariableTree) parent);
112149
}
113150
return false;
114151
}
115152

116-
static boolean isTryStatementResource(VariableTree variable) {
153+
private static boolean isTryStatementResource(VariableTree variable) {
117154
final TryStatementTree tryStatement = getEnclosingTryStatement(variable);
118-
if (tryStatement != null) {
119-
return tryStatement.resources().contains(variable);
120-
}
121-
return false;
155+
return tryStatement != null && tryStatement.resources().contains(variable);
122156
}
123157

124158
private static TryStatementTree getEnclosingTryStatement(Tree syntaxNode) {
@@ -132,48 +166,11 @@ private static TryStatementTree getEnclosingTryStatement(Tree syntaxNode) {
132166
return null;
133167
}
134168

135-
private static class PostStatementVisitor extends CheckerTreeNodeVisitor {
136-
137-
public PostStatementVisitor(CheckerContext context) {
138-
super(context.getState());
139-
}
140-
141-
@Override
142-
public void visitNewClass(NewClassTree syntaxNode) {
143-
if (isOpeningResource(syntaxNode)) {
144-
final SymbolicValue instanceValue = programState.peekValue();
145-
if (!(instanceValue instanceof ResourceWrapperSymbolicValue)) {
146-
programState = programState.addConstraint(instanceValue, new ObjectConstraint(false, false, syntaxNode, Status.OPENED));
147-
}
148-
}
149-
}
150-
151-
@Override
152-
public void visitMethodInvocation(MethodInvocationTree syntaxNode) {
153-
if (syntaxNode.methodSelect().is(Tree.Kind.MEMBER_SELECT) && needsClosing(syntaxNode.symbolType())) {
154-
final ExpressionTree targetExpression = ((MemberSelectExpressionTree) syntaxNode.methodSelect()).expression();
155-
if (targetExpression.is(Tree.Kind.IDENTIFIER) && !isWithinTryHeader(syntaxNode)
156-
&& (syntaxNode.symbol().isStatic() || isJdbcResourceCreation(targetExpression))) {
157-
programState = programState.addConstraint(programState.peekValue(), new ObjectConstraint(false, false, syntaxNode, Status.OPENED));
158-
}
159-
}
160-
}
161-
162-
private static boolean isJdbcResourceCreation(ExpressionTree targetExpression) {
163-
for (String creator : JDBC_RESOURCE_CREATIONS) {
164-
if (targetExpression.symbolType().is(creator)) {
165-
return true;
166-
}
167-
}
168-
return false;
169-
}
170-
}
171-
172-
public static class ResourceWrapperSymbolicValue extends SymbolicValue {
169+
private static class ResourceWrapperSymbolicValue extends SymbolicValue {
173170

174171
private final SymbolicValue dependent;
175172

176-
public ResourceWrapperSymbolicValue(int id, SymbolicValue dependent) {
173+
ResourceWrapperSymbolicValue(int id, SymbolicValue dependent) {
177174
super(id);
178175
this.dependent = dependent;
179176
}
@@ -182,9 +179,10 @@ public ResourceWrapperSymbolicValue(int id, SymbolicValue dependent) {
182179
public SymbolicValue wrappedValue() {
183180
return dependent.wrappedValue();
184181
}
182+
185183
}
186184

187-
static class WrappedValueFactory implements SymbolicValueFactory {
185+
private static class WrappedValueFactory implements SymbolicValueFactory {
188186

189187
private final SymbolicValue value;
190188

@@ -196,12 +194,13 @@ static class WrappedValueFactory implements SymbolicValueFactory {
196194
public SymbolicValue createSymbolicValue(int counter, Tree syntaxNode) {
197195
return new ResourceWrapperSymbolicValue(counter, value);
198196
}
199-
}
200197

201-
static class PreStatementVisitor extends CheckerTreeNodeVisitor {
198+
}
202199

200+
private static class PreStatementVisitor extends CheckerTreeNodeVisitor {
203201
private static final String CLOSE_METHOD_NAME = "close";
204202
private static final String GET_MORE_RESULTS_METHOD_NAME = "getMoreResults";
203+
205204
private static final String GET_RESULT_SET = "getResultSet";
206205

207206
private final ConstraintManager constraintManager;
@@ -298,12 +297,11 @@ private SymbolicValue getTargetValue(MethodInvocationTree syntaxNode) {
298297
}
299298

300299
private void closeResultSetsRelatedTo(SymbolicValue value) {
301-
final List<ConstrainedValue> constrainedValues = programState.getValuesWithConstraints(Status.OPENED);
302-
for (ConstrainedValue constrainedValue : constrainedValues) {
303-
if (constrainedValue.value instanceof ResourceWrapperSymbolicValue) {
304-
ResourceWrapperSymbolicValue rValue = (ResourceWrapperSymbolicValue) constrainedValue.value;
300+
for (Map.Entry<SymbolicValue, ObjectConstraint> constrainedValue : programState.getValuesWithConstraints(Status.OPENED).entrySet()) {
301+
if (constrainedValue.getKey() instanceof ResourceWrapperSymbolicValue) {
302+
ResourceWrapperSymbolicValue rValue = (ResourceWrapperSymbolicValue) constrainedValue.getKey();
305303
if (value.equals(rValue.dependent)) {
306-
programState = programState.addConstraint(rValue, constrainedValue.constraint.withStatus(Status.CLOSED));
304+
programState = programState.addConstraint(rValue, constrainedValue.getValue().withStatus(Status.CLOSED));
307305
}
308306
}
309307
}
@@ -338,55 +336,59 @@ private static ObjectConstraint openedConstraint(ProgramState programState, Symb
338336
return null;
339337
}
340338

341-
private static boolean isClosingResource(Symbol methodInvocationSymbol) {
342-
return methodInvocationSymbol.isMethodSymbol() && CLOSE_METHOD_NAME.equals(methodInvocationSymbol.name());
339+
private static boolean isClosingResource(Symbol symbol) {
340+
return isMethodMatchingName(symbol, CLOSE_METHOD_NAME);
343341
}
344342

345-
private static boolean isClosingResultSets(Symbol methodInvocationSymbol) {
346-
return methodInvocationSymbol.isMethodSymbol() && GET_MORE_RESULTS_METHOD_NAME.equals(methodInvocationSymbol.name());
343+
private static boolean isClosingResultSets(Symbol symbol) {
344+
return isMethodMatchingName(symbol, GET_MORE_RESULTS_METHOD_NAME);
347345
}
348346

349-
private static boolean isOpeningResultSet(Symbol methodInvocationSymbol) {
350-
return methodInvocationSymbol.isMethodSymbol() && GET_RESULT_SET.equals(methodInvocationSymbol.name());
347+
private static boolean isOpeningResultSet(Symbol symbol) {
348+
return isMethodMatchingName(symbol, GET_RESULT_SET);
351349
}
352-
}
353350

354-
@Override
355-
public void scanFile(JavaFileScannerContext context) {
356-
Multimap<Tree, String> issues = ((DefaultJavaFileScannerContext) context).getSEIssues(UnclosedResourcesCheck.class);
357-
for (Map.Entry<Tree, String> issue : issues.entries()) {
358-
context.reportIssue(this, issue.getKey(), issue.getValue());
351+
private static boolean isMethodMatchingName(Symbol symbol, String matchName) {
352+
return symbol.isMethodSymbol() && matchName.equals(symbol.name());
359353
}
360-
}
361354

362-
@Override
363-
public ProgramState checkPreStatement(CheckerContext context, Tree syntaxNode) {
364-
final PreStatementVisitor visitor = new PreStatementVisitor(context);
365-
syntaxNode.accept(visitor);
366-
return visitor.programState;
367355
}
368356

369-
@Override
370-
public ProgramState checkPostStatement(CheckerContext context, Tree syntaxNode) {
371-
final PostStatementVisitor visitor = new PostStatementVisitor(context);
372-
syntaxNode.accept(visitor);
373-
return visitor.programState;
374-
}
357+
private static class PostStatementVisitor extends CheckerTreeNodeVisitor {
375358

376-
@Override
377-
public void checkEndOfExecutionPath(CheckerContext context, ConstraintManager constraintManager) {
378-
final List<ObjectConstraint> constraints = context.getState().getFieldConstraints(Status.OPENED);
379-
for (ObjectConstraint constraint : constraints) {
380-
Tree syntaxNode = constraint.syntaxNode();
381-
if (syntaxNode.is(Tree.Kind.NEW_CLASS)) {
382-
context.reportIssue(syntaxNode, this, "Close this \"" + ((NewClassTree) syntaxNode).identifier() + "\".");
383-
} else if (syntaxNode.is(Tree.Kind.METHOD_INVOCATION)) {
384-
context.reportIssue(syntaxNode, this, "Close this \"" + toString((MethodInvocationTree) syntaxNode) + "\".");
359+
PostStatementVisitor(CheckerContext context) {
360+
super(context.getState());
361+
}
362+
363+
@Override
364+
public void visitNewClass(NewClassTree syntaxNode) {
365+
if (isOpeningResource(syntaxNode)) {
366+
final SymbolicValue instanceValue = programState.peekValue();
367+
if (!(instanceValue instanceof ResourceWrapperSymbolicValue)) {
368+
programState = programState.addConstraint(instanceValue, new ObjectConstraint(false, false, syntaxNode, Status.OPENED));
369+
}
370+
}
371+
}
372+
373+
@Override
374+
public void visitMethodInvocation(MethodInvocationTree syntaxNode) {
375+
if (syntaxNode.methodSelect().is(Tree.Kind.MEMBER_SELECT) && needsClosing(syntaxNode.symbolType())) {
376+
final ExpressionTree targetExpression = ((MemberSelectExpressionTree) syntaxNode.methodSelect()).expression();
377+
if (targetExpression.is(Tree.Kind.IDENTIFIER) && !isWithinTryHeader(syntaxNode)
378+
&& (syntaxNode.symbol().isStatic() || isJdbcResourceCreation(targetExpression))) {
379+
programState = programState.addConstraint(programState.peekValue(), new ObjectConstraint(false, false, syntaxNode, Status.OPENED));
380+
}
385381
}
386382
}
387-
}
388383

389-
private static String toString(MethodInvocationTree syntaxNode) {
390-
return syntaxNode.symbolType().name();
384+
private static boolean isJdbcResourceCreation(ExpressionTree targetExpression) {
385+
for (String creator : JDBC_RESOURCE_CREATIONS) {
386+
if (targetExpression.symbolType().is(creator)) {
387+
return true;
388+
}
389+
}
390+
return false;
391+
}
391392
}
393+
392394
}

0 commit comments

Comments
 (0)