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.

Sign up for GitHub

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

Jump to bottom

remove comments from same line #195

Merged
merged 2 commits into from
Oct 19, 2023
Merged

remove comments from same line #195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af95fce
remove comments from same line
Oct 18, 2023
dea158e
added language highlighting in tests
Oct 18, 2023
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
87 changes: 76 additions & 11 deletions src/main/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateFields.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@
import org.openrewrite.ExecutionContext;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.Space;
import org.openrewrite.java.tree.Statement;
import org.openrewrite.java.tree.TypeUtils;

import java.time.Duration;
import java.util.*;
import java.util.concurrent.atomic.AtomicBoolean;

@Value
@EqualsAndHashCode(callSuper = true)
Expand Down Expand Up @@ -57,23 +60,32 @@ public Duration getEstimatedEffortPerOccurrence() {
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new JavaIsoVisitor<ExecutionContext>() {
@Value
class CheckField {
J.VariableDeclarations declarations;
@Nullable Statement nextStatement;
}

@Override
public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext executionContext) {
J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, executionContext);

List<J.VariableDeclarations> checkFields = new ArrayList<>();
List<CheckField> checkFields = new ArrayList<>();
// Do not remove fields with `serialVersionUID` name.
boolean skipSerialVersionUID = cd.getType() == null ||
cd.getType().isAssignableTo("java.io.Serializable");
for (Statement statement : cd.getBody().getStatements()) {

List<Statement> statements = cd.getBody().getStatements();
for (int i = 0; i < statements.size(); i++) {
Statement statement = statements.get(i);
if (statement instanceof J.VariableDeclarations) {
J.VariableDeclarations vd = (J.VariableDeclarations) statement;
// RSPEC-1068 does not apply serialVersionUID of Serializable classes, or fields with annotations.
if (!(skipSerialVersionUID && isSerialVersionUid(vd)) &&
vd.getLeadingAnnotations().isEmpty() &&
vd.hasModifier(J.Modifier.Type.Private)) {
checkFields.add(vd);
Statement nextStatement = i < statements.size() - 1 ? statements.get(i + 1) : null;
checkFields.add(new CheckField(vd, nextStatement));
}
} else if (statement instanceof J.MethodDeclaration) {
// RSPEC-1068 does not apply fields from classes with native methods.
Expand All @@ -94,12 +106,18 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex
outer = parent.getValue();
}
}
for (J.VariableDeclarations fields : checkFields) {
for (CheckField checkField : checkFields) {
// Find variable uses.
Map<J.VariableDeclarations.NamedVariable, List<J.Identifier>> inUse = VariableUses.find(fields, outer);
Map<J.VariableDeclarations.NamedVariable, List<J.Identifier>> inUse =
VariableUses.find(checkField.declarations, outer);
for (Map.Entry<J.VariableDeclarations.NamedVariable, List<J.Identifier>> entry : inUse.entrySet()) {
if (entry.getValue().isEmpty()) {
cd = (J.ClassDeclaration) new RemoveUnusedField(entry.getKey()).visitNonNull(cd, executionContext);
AtomicBoolean declarationDeleted = new AtomicBoolean();
cd = (J.ClassDeclaration) new RemoveUnusedField(entry.getKey()).visitNonNull(cd, declarationDeleted);
// Maybe remove next statement comment if variable declarations is removed
if (declarationDeleted.get()) {
cd = (J.ClassDeclaration) new MaybeRemoveComment(checkField.nextStatement, cd).visitNonNull(cd, executionContext);
}
}
}
}
Expand Down Expand Up @@ -159,29 +177,76 @@ public J.Identifier visitIdentifier(J.Identifier identifier,
}
}

private static class RemoveUnusedField extends JavaVisitor<ExecutionContext> {
private static class RemoveUnusedField extends JavaVisitor<AtomicBoolean> {
private final J.VariableDeclarations.NamedVariable namedVariable;

public RemoveUnusedField(J.VariableDeclarations.NamedVariable namedVariable) {
this.namedVariable = namedVariable;
}

@Override
public J visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext executionContext) {
public J visitVariableDeclarations(J.VariableDeclarations multiVariable, AtomicBoolean declarationDeleted) {
if (multiVariable.getVariables().size() == 1 && multiVariable.getVariables().contains(namedVariable)) {
declarationDeleted.set(true);
//noinspection ConstantConditions
return null;
}
return super.visitVariableDeclarations(multiVariable, executionContext);
return super.visitVariableDeclarations(multiVariable, declarationDeleted);
}

@Override
public J visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext executionContext) {
public J visitVariable(J.VariableDeclarations.NamedVariable variable, AtomicBoolean declarationDeleted) {
if (variable == namedVariable) {
//noinspection ConstantConditions
return null;
}
return super.visitVariable(variable, executionContext);
return super.visitVariable(variable, declarationDeleted);
}
}

private static class MaybeRemoveComment extends JavaVisitor<ExecutionContext> {
@Nullable
private final Statement statement;
private final J.ClassDeclaration classDeclaration;

public MaybeRemoveComment(@Nullable Statement statement, J.ClassDeclaration classDeclaration) {
this.statement = statement;
this.classDeclaration = classDeclaration;
}

@Override
public J visitStatement(Statement s, ExecutionContext executionContext) {
if (s == statement) {
Space prefix = s.getPrefix();
// If we have at least one comment and there is no newline
if (prefix.getComments().size() > 0 && !prefix.getWhitespace().contains("\n")) {
return s.withPrefix(prefix
// Copy suffix to prefix
.withWhitespace(prefix.getComments().get(0).getSuffix())
// Remove the first comment
.withComments(prefix.getComments().subList(1, prefix.getComments().size())
));

}
}
return super.visitStatement(s, executionContext);
}

@Override
public J visitClassDeclaration(J.ClassDeclaration c, ExecutionContext executionContext) {
// We also need to remove comments attached to end of classDeclaration if it's the last statement
if (statement == null && c == classDeclaration) {
Space end = c.getBody().getEnd();
// If we have at least one comment and there is no newline
if (end.getComments().size() > 0 && !end.getWhitespace().contains("\n")) {
return c.withBody(c.getBody().withEnd(end
.withWhitespace(end.getComments().get(0).getSuffix())
.withComments(end.getComments().subList(1, end.getComments().size()))
));
}
}
return super.visitClassDeclaration(c, executionContext);
}
}

}
148 changes: 148 additions & 0 deletions src/test/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateFieldsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,5 +224,153 @@ public doSomethingWithAVehicle() {
)
);
}

@Test
void removeCommentsPrefix() {
rewriteRun(
//language=java
java(
"""
public class Test {
// Some comment
private int a;
}
""", """
public class Test {
}
"""
)
);
}
@Test
void removeCommentsLastExpression() {
rewriteRun(
//language=java
java(
"""
public class Test {
private int a; // Some comment
}
""", """
public class Test {
}
"""
)
);
}

@Test
void removeCommentsSameLine() {
rewriteRun(
//language=java
java(
"""
public class Test {
private int a;
private int b; // Some comment

public void test() {
a = 42;
}
}
""", """
public class Test {
private int a;

public void test() {
a = 42;
}
}
"""
)
);
}

@Test
void removeCommentsMultiLine() {
rewriteRun(
//language=java
java(
"""
public class Test {
private int a;
private int b; /*
Some
multiline
comment
*/

public void test() {
a = 42;
}
}
""", """
public class Test {
private int a;

public void test() {
a = 42;
}
}
"""
)
);

}

@Test
void doNotRemoveCommentsIfNewline() {
rewriteRun(
//language=java
java(
"""
public class Test {
private int a;
private int b;
// Some comment

public void test() {
a = 42;
}
}
""", """
public class Test {
private int a;
// Some comment

public void test() {
a = 42;
}
}
"""
)
);
}

@Test
void doNotRemoveCommentsIfNotRemovedWholeVariableDeclarations() {
rewriteRun(
//language=java
java(
"""
public class Test {
private int a, b; // Some comment

public void test() {
a = 42;
}
}
""", """
public class Test {
private int a; // Some comment

public void test() {
a = 42;
}
}
"""
)
);
}
}

Loading