Skip to content

Commit

Permalink
Use var in MustBeClosedChecker suggestions
Browse files Browse the repository at this point in the history
Instead of writing out a specific type, we'll just use `var`. If we already have a declaration, e.g. replacing

  Foo f = foo();

with

  try (Foo f = foo()) { ... }

we still use the existing syntax (i.e., Foo, not var).

PiperOrigin-RevId: 472498888
  • Loading branch information
amalloy authored and Error Prone Team committed Sep 6, 2022
1 parent 6026393 commit 5a0b669
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -356,10 +356,6 @@ protected void addFix(

private Optional<TryBlock> introduceSingleStatementTry(
ExpressionTree tree, StatementTree stmt, VisitorState state) {
Type type = getType(tree);
if (type == null) {
return Optional.empty();
}
SuggestedFix.Builder fix = SuggestedFix.builder();
String name = suggestName(tree);
if (state.getPath().getParentPath().getLeaf() instanceof ExpressionStatementTree) {
Expand All @@ -371,10 +367,7 @@ private Optional<TryBlock> introduceSingleStatementTry(
new TryBlock(
stmt,
fix.prefixWith(
stmt,
String.format(
"try (%s %s = %s) {",
qualifyType(state, fix, type), name, state.getSourceForNode(tree)))));
stmt, String.format("try (var %s = %s) {", name, state.getSourceForNode(tree)))));
}

private Optional<TryBlock> extractToResourceInCurrentTry(
Expand All @@ -397,10 +390,6 @@ private Optional<TryBlock> extractToResourceInCurrentTry(

private Optional<TryBlock> splitVariableDeclarationAroundTry(
ExpressionTree tree, VariableTree var, VisitorState state) {
Type type = getType(tree);
if (type == null) {
return Optional.empty();
}
int initPos = getStartPosition(var.getInitializer());
int afterTypePos = state.getEndPosition(var.getType());
String name = suggestName(tree);
Expand All @@ -412,12 +401,8 @@ private Optional<TryBlock> splitVariableDeclarationAroundTry(
afterTypePos,
initPos,
String.format(
" %s;\ntry (%s %s = %s) {\n%s =",
var.getName(),
qualifyType(state, fix, type),
name,
state.getSourceForNode(tree),
var.getName()))
" %s;\ntry (var %s = %s) {\n%s =",
var.getName(), name, state.getSourceForNode(tree), var.getName()))
.replace(tree, name)));
}

Expand All @@ -436,7 +421,7 @@ private Optional<TryBlock> wrapTryFinallyAroundVariableScope(
String.format(
"try (%s %s = %s) {",
state.getSourceForNode(decl.getType()),
decl.getName().toString(),
decl.getName(),
state.getSourceForNode(decl.getInitializer())))
.delete(decl)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Closeable mustBeClosedAnnotatedMethod() {

void sameClass() {
// BUG: Diagnostic contains:
try (Closeable closeable = mustBeClosedAnnotatedMethod()) {}
try (var closeable = mustBeClosedAnnotatedMethod()) {}
}
}

Expand All @@ -64,14 +64,13 @@ class MustBeClosedAnnotatedConstructor extends Closeable {

void sameClass() {
// BUG: Diagnostic contains:
try (MustBeClosedAnnotatedConstructor mustBeClosedAnnotatedConstructor =
new MustBeClosedAnnotatedConstructor()) {}
try (var mustBeClosedAnnotatedConstructor = new MustBeClosedAnnotatedConstructor()) {}
}
}

void positiveCase1() {
// BUG: Diagnostic contains:
try (Closeable closeable = new Foo().mustBeClosedAnnotatedMethod()) {}
try (var closeable = new Foo().mustBeClosedAnnotatedMethod()) {}
}

void positiveCase2() {
Expand All @@ -82,22 +81,21 @@ void positiveCase2() {
void positiveCase3() {
try {
// BUG: Diagnostic contains:
try (Closeable closeable = new Foo().mustBeClosedAnnotatedMethod()) {}
try (var closeable = new Foo().mustBeClosedAnnotatedMethod()) {}
} finally {
}
}

void positiveCase4() {
try (Closeable c = new Foo().mustBeClosedAnnotatedMethod()) {
// BUG: Diagnostic contains:
try (Closeable closeable = new Foo().mustBeClosedAnnotatedMethod()) {}
try (var closeable = new Foo().mustBeClosedAnnotatedMethod()) {}
}
}

void positiveCase5() {
// BUG: Diagnostic contains:
try (MustBeClosedAnnotatedConstructor mustBeClosedAnnotatedConstructor =
new MustBeClosedAnnotatedConstructor()) {}
try (var mustBeClosedAnnotatedConstructor = new MustBeClosedAnnotatedConstructor()) {}
}

@MustBeClosed
Expand Down Expand Up @@ -152,23 +150,23 @@ public Closeable mustBeClosedAnnotatedMethod() {

void subexpression() {
// BUG: Diagnostic contains:
try (Closeable closeable = new Foo().mustBeClosedAnnotatedMethod()) {
try (var closeable = new Foo().mustBeClosedAnnotatedMethod()) {
closeable.method();
}
}

void ternary(boolean condition) {
// BUG: Diagnostic contains:
int result;
try (Closeable closeable = new Foo().mustBeClosedAnnotatedMethod()) {
try (var closeable = new Foo().mustBeClosedAnnotatedMethod()) {
result = condition ? closeable.method() : 0;
}
}

int variableDeclaration() {
// BUG: Diagnostic contains:
int result;
try (Closeable closeable = new Foo().mustBeClosedAnnotatedMethod()) {
try (var closeable = new Foo().mustBeClosedAnnotatedMethod()) {
result = closeable.method();
}
return result;
Expand All @@ -182,7 +180,7 @@ void forLoopInitialization() {
void forLoopConditionUnfixable() {
// TODO(b/236715080): suggested fix changes behavior.
// BUG: Diagnostic contains:
try (final Closeable closeable = new Foo().mustBeClosedAnnotatedMethod()) {
try (final var closeable = new Foo().mustBeClosedAnnotatedMethod()) {
for (int i = 0; i < closeable.method(); ++i) {}
}
}
Expand Down

0 comments on commit 5a0b669

Please sign in to comment.