BulkFileAttributesReadInspection: clustering attribute calls by loops before reporting (IJ-CR-20279)

GitOrigin-RevId: 70cebc3298eea9feb57f7b4f1f4ca7b8b0535c66
This commit is contained in:
Artemiy Sartakov
2022-02-14 13:16:26 +07:00
committed by intellij-monorepo-bot
parent 1df9c064f4
commit 6633bef908
12 changed files with 232 additions and 59 deletions

View File

@@ -24,6 +24,7 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.*;
import java.util.function.Consumer;
public class BulkFileAttributesReadInspection extends AbstractBaseJavaLocalInspectionTool {
private static final Map<String, String> ATTR_REPLACEMENTS = Map.of(
@@ -46,47 +47,14 @@ public class BulkFileAttributesReadInspection extends AbstractBaseJavaLocalInspe
super.visitMethod(method);
PsiCodeBlock methodBody = method.getBody();
if (methodBody == null) return;
AttributeCallsCollector callsCollector = new AttributeCallsCollector(method);
methodBody.accept(callsCollector);
Map<PsiVariable, List<PsiMethodCallExpression>> attributeCalls = callsCollector.getCalls();
attributeCalls.forEach((var, attrCalls) -> {
if (distinctCalls(attrCalls) < 2) return;
PsiExpression[] occurrences = attrCalls.toArray(PsiExpression.EMPTY_ARRAY);
PsiElement anchor = CommonJavaRefactoringUtil.getAnchorElementForMultipleExpressions(occurrences, method);
if (anchor == null) return;
boolean needsTryCatchBlock = needsTryCatchBlock(anchor);
if (!needsTryCatchBlock) {
attrCalls.forEach(call -> holder.registerProblem(call, JavaBundle.message("inspection.bulk.file.attributes.read.message"),
new ReplaceWithBulkCallFix(isOnTheFly)));
return;
}
if (isOnTheFly) {
attrCalls.forEach(call -> holder.registerProblem(call, JavaBundle.message("inspection.bulk.file.attributes.read.message"),
ProblemHighlightType.INFORMATION,
new ReplaceWithBulkCallFix(true)));
}
});
CallReporter reporter = new CallReporter(methodBody, isOnTheFly, holder);
FileAttributeCallsVisitor visitor = new FileAttributeCallsVisitor(methodBody, reporter);
methodBody.accept(visitor);
reporter.accept(visitor.myCalls);
}
};
}
@Nullable
private static PsiVariable getFileVariable(@NotNull PsiMethodCallExpression call) {
PsiReferenceExpression qualifier = ObjectUtils.tryCast(call.getMethodExpression().getQualifier(), PsiReferenceExpression.class);
if (qualifier == null) return null;
return ObjectUtils.tryCast(qualifier.resolve(), PsiVariable.class);
}
@NotNull
private static List<PsiMethodCallExpression> findAttributeCalls(@NotNull PsiVariable fileVar, @NotNull PsiElement scope) {
Collection<PsiMethodCallExpression> calls = ReferencesSearch.search(fileVar, new LocalSearchScope(scope))
.mapping(ref -> ObjectUtils.tryCast(ref, PsiReferenceExpression.class))
.mapping(ref -> ref == null ? null : ExpressionUtils.getCallForQualifier(ref))
.filtering(FILE_ATTR_CALL_MATCHER)
.findAll();
return new ArrayList<>(calls);
}
private static long distinctCalls(@NotNull List<PsiMethodCallExpression> calls) {
return StreamEx.of(calls).distinct(call -> call.getMethodExpression().getReferenceName()).count();
}
@@ -96,13 +64,78 @@ public class BulkFileAttributesReadInspection extends AbstractBaseJavaLocalInspe
return !ExceptionUtil.isHandled(ioExceptionType, anchor);
}
private static class AttributeCallsCollector extends JavaRecursiveElementWalkingVisitor {
private static class CallReporter implements Consumer<Map<PsiVariable, List<PsiMethodCallExpression>>> {
private final PsiElement myScope;
private final boolean myIsOnTheFly;
private final ProblemsHolder myHolder;
private CallReporter(@NotNull PsiElement scope, boolean isOnTheFly, @NotNull ProblemsHolder holder) {
myScope = scope;
myIsOnTheFly = isOnTheFly;
myHolder = holder;
}
@Override
public void accept(Map<PsiVariable, List<PsiMethodCallExpression>> calls) {
calls.forEach((variable, varCalls) -> {
if (distinctCalls(varCalls) < 2) return;
PsiExpression[] occurrences = varCalls.toArray(PsiExpression.EMPTY_ARRAY);
PsiElement anchor = CommonJavaRefactoringUtil.getAnchorElementForMultipleExpressions(occurrences, myScope);
if (anchor == null) return;
boolean needsTryCatchBlock = needsTryCatchBlock(anchor);
if (!needsTryCatchBlock) {
varCalls.forEach(call -> myHolder.registerProblem(call, JavaBundle.message("inspection.bulk.file.attributes.read.message"),
new ReplaceWithBulkCallFix(myIsOnTheFly)));
return;
}
if (myIsOnTheFly) {
varCalls.forEach(call -> myHolder.registerProblem(call, JavaBundle.message("inspection.bulk.file.attributes.read.message"),
ProblemHighlightType.INFORMATION,
new ReplaceWithBulkCallFix(true)));
}
});
}
}
private static class FileAttributeCallsVisitor extends JavaRecursiveElementWalkingVisitor {
private final PsiElement myScope;
private final Map<PsiVariable, List<PsiMethodCallExpression>> myCalls = new HashMap<>();
private final Consumer<Map<PsiVariable, List<PsiMethodCallExpression>>> myReporter;
private AttributeCallsCollector(PsiElement scope) {
private FileAttributeCallsVisitor(@NotNull PsiElement scope,
@NotNull Consumer<Map<PsiVariable, List<PsiMethodCallExpression>>> reporter) {
myScope = scope;
myReporter = reporter;
}
@Override
public void visitForStatement(PsiForStatement statement) {
doVisitLoop(statement);
}
@Override
public void visitWhileStatement(PsiWhileStatement statement) {
doVisitLoop(statement);
}
@Override
public void visitForeachStatement(PsiForeachStatement statement) {
doVisitLoop(statement);
}
@Override
public void visitDoWhileStatement(PsiDoWhileStatement statement) {
doVisitLoop(statement);
}
private void doVisitLoop(@NotNull PsiLoopStatement loop) {
PsiStatement loopBody = loop.getBody();
if (loopBody == null) return;
FileAttributeCallsVisitor visitor = new FileAttributeCallsVisitor(myScope, myReporter);
loopBody.accept(visitor);
myReporter.accept(visitor.myCalls);
}
@Override
@@ -114,13 +147,9 @@ public class BulkFileAttributesReadInspection extends AbstractBaseJavaLocalInspe
PsiVariable variable = ObjectUtils.tryCast(ref.resolve(), PsiVariable.class);
if (variable == null) return;
if (!HighlightControlFlowUtil.isEffectivelyFinal(variable, myScope, null)) return;
List<PsiMethodCallExpression> varCalls = myCalls.computeIfAbsent(variable, k -> new SmartList<>());
List<PsiMethodCallExpression> varCalls = myCalls.computeIfAbsent(variable, __ -> new SmartList<>());
varCalls.add(call);
}
private Map<PsiVariable, List<PsiMethodCallExpression>> getCalls() {
return myCalls;
}
}
private static class ReplaceWithBulkCallFix implements LocalQuickFix {
@@ -148,7 +177,7 @@ public class BulkFileAttributesReadInspection extends AbstractBaseJavaLocalInspe
if (parent == null) return;
String fileVarName = fileVariable.getName();
if (fileVarName == null) return;
AttributesVariableModel attributesVariable = AttributesVariableModel.create(fileVarName, fileVariable.myContainingMethod, anchor);
AttributesVariableModel attributesVariable = AttributesVariableModel.create(fileVarName, fileVariable.myScope, anchor);
if (attributesVariable == null) return;
final PsiDeclarationStatement declaration;
@@ -179,7 +208,7 @@ public class BulkFileAttributesReadInspection extends AbstractBaseJavaLocalInspe
if (!myIsOnTheFly) return;
PsiVariable attrsVariable = (PsiVariable)declaration.getDeclaredElements()[0];
HighlightUtils.showRenameTemplate(fileVariable.myContainingMethod, attrsVariable, usages);
HighlightUtils.showRenameTemplate(fileVariable.myScope, attrsVariable, usages);
}
private static @NotNull PsiExpressionStatement addAssignment(@NotNull PsiElement parent,
@@ -236,14 +265,14 @@ public class BulkFileAttributesReadInspection extends AbstractBaseJavaLocalInspe
private static class FileVariableModel {
private final PsiVariable myFileVariable;
private final PsiMethod myContainingMethod;
private final PsiElement myScope;
private List<PsiMethodCallExpression> myAttributeCalls;
private FileVariableModel(@NotNull List<PsiMethodCallExpression> attributeCalls,
@NotNull PsiVariable fileVariable, @NotNull PsiMethod containingMethod) {
@NotNull PsiVariable fileVariable, @NotNull PsiElement scope) {
myAttributeCalls = attributeCalls;
myFileVariable = fileVariable;
myContainingMethod = containingMethod;
myScope = scope;
}
private @Nullable String getName() {
@@ -252,24 +281,49 @@ public class BulkFileAttributesReadInspection extends AbstractBaseJavaLocalInspe
private @Nullable PsiElement findAnchor() {
PsiExpression[] occurrences = myAttributeCalls.toArray(PsiExpression.EMPTY_ARRAY);
PsiElement anchor = CommonJavaRefactoringUtil.getAnchorElementForMultipleExpressions(occurrences, myContainingMethod);
PsiElement anchor = CommonJavaRefactoringUtil.getAnchorElementForMultipleExpressions(occurrences, myScope);
if (anchor == null) return null;
PsiLambdaExpression lambda = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprUp(anchor.getParent()), PsiLambdaExpression.class);
if (lambda == null) return anchor;
PsiCodeBlock codeBlock = CommonJavaRefactoringUtil.expandExpressionLambdaToCodeBlock(lambda);
// attribute calls were inside lambda, need to recalculate them
myAttributeCalls = findAttributeCalls(myFileVariable, myContainingMethod);
myAttributeCalls = findAttributeCalls(myFileVariable, myScope);
return ControlFlowUtils.getOnlyStatementInBlock(codeBlock);
}
@NotNull
private static List<PsiMethodCallExpression> findAttributeCalls(@NotNull PsiVariable fileVar, @NotNull PsiElement scope) {
Collection<PsiMethodCallExpression> calls = ReferencesSearch.search(fileVar, new LocalSearchScope(scope))
.filtering(ref -> scope == findScope(ref.getElement()))
.mapping(ref -> ObjectUtils.tryCast(ref, PsiReferenceExpression.class))
.mapping(ref -> ref == null ? null : ExpressionUtils.getCallForQualifier(ref))
.filtering(FILE_ATTR_CALL_MATCHER)
.findAll();
return new ArrayList<>(calls);
}
private static @Nullable FileVariableModel create(@NotNull PsiMethodCallExpression variableUsage) {
PsiMethod containingMethod = PsiTreeUtil.getParentOfType(variableUsage, PsiMethod.class);
if (containingMethod == null) return null;
PsiElement scope = findScope(variableUsage);
if (scope == null) return null;
PsiVariable psiVariable = getFileVariable(variableUsage);
if (psiVariable == null) return null;
List<PsiMethodCallExpression> attributeCalls = findAttributeCalls(psiVariable, containingMethod);
List<PsiMethodCallExpression> attributeCalls = findAttributeCalls(psiVariable, scope);
if (distinctCalls(attributeCalls) < 2) return null;
return new FileVariableModel(attributeCalls, psiVariable, containingMethod);
return new FileVariableModel(attributeCalls, psiVariable, scope);
}
@Nullable
private static PsiVariable getFileVariable(@NotNull PsiMethodCallExpression call) {
PsiReferenceExpression qualifier = ObjectUtils.tryCast(call.getMethodExpression().getQualifier(), PsiReferenceExpression.class);
if (qualifier == null) return null;
return ObjectUtils.tryCast(qualifier.resolve(), PsiVariable.class);
}
@Nullable
private static PsiElement findScope(@NotNull PsiElement element) {
PsiElement scope = PsiTreeUtil.getParentOfType(element, PsiMethod.class, PsiLoopStatement.class);
if (scope instanceof PsiLoopStatement) scope = ((PsiLoopStatement)scope).getBody();
return scope;
}
}

View File

@@ -0,0 +1,21 @@
// "Replace with bulk 'Files.readAttributes' call" "true"
import java.io.*;
import java.nio.file.Files;
import java.nio.file.attribute.BasicFileAttributes;
class Foo {
long isNewFile(File file, long lastModified) {
System.out.println(file.isFile());
while (file.isDirectory()) {
BasicFileAttributes basicFileAttributes;
try {
basicFileAttributes = Files.readAttributes(file.toPath(), BasicFileAttributes.class);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
System.out.println(basicFileAttributes.isRegularFile());
System.out.println(basicFileAttributes.size());
}
return file.lastModified();
}
}

View File

@@ -0,0 +1,21 @@
// "Replace with bulk 'Files.readAttributes' call" "true"
import java.io.*;
import java.nio.file.Files;
import java.nio.file.attribute.BasicFileAttributes;
class Foo {
void printWhileDirectory(File file) {
BasicFileAttributes basicFileAttributes;
try {
basicFileAttributes = Files.readAttributes(file.toPath(), BasicFileAttributes.class);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
System.out.println(basicFileAttributes.isRegularFile());
System.out.println(basicFileAttributes.lastModifiedTime().toMillis());
while (file.isDirectory()) {
System.out.println(file.isFile());
System.out.println(file.length());
}
}
}

View File

@@ -0,0 +1,23 @@
// "Replace with bulk 'Files.readAttributes' call" "true"
import java.io.*;
import java.nio.file.Files;
import java.nio.file.attribute.BasicFileAttributes;
class Foo {
long isNewFile(File file, long lastModified) {
while (file.isDirectory()) {
System.out.println(file.isFile());
for (int i = 0; i < 10 && file.isDirectory(); i++) {
BasicFileAttributes basicFileAttributes;
try {
basicFileAttributes = Files.readAttributes(file.toPath(), BasicFileAttributes.class);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
System.out.println(basicFileAttributes.isRegularFile());
System.out.println(basicFileAttributes.size());
}
}
return file.lastModified();
}
}

View File

@@ -6,7 +6,7 @@ import java.nio.file.attribute.BasicFileAttributes;
class Foo {
long isNewFile(File file) throws IOException {
BasicFileAttributes basicFileAttributes = Files.readAttributes(file.toPath(), BasicFileAttributes.class);
while (basicFileAttributes.isDirectory()) {
if (basicFileAttributes.isDirectory()) {
System.out.println(basicFileAttributes.isRegularFile());
}
return basicFileAttributes.lastModifiedTime().toMillis();

View File

@@ -11,7 +11,7 @@ class Foo {
} catch (IOException e) {
throw new UncheckedIOException(e);
}
while (basicFileAttributes.isDirectory()) {
if (basicFileAttributes.isDirectory()) {
System.out.println(basicFileAttributes.isRegularFile());
}
return basicFileAttributes.lastModifiedTime().toMillis();

View File

@@ -0,0 +1,13 @@
// "Replace with bulk 'Files.readAttributes' call" "true"
import java.io.*;
class Foo {
long isNewFile(File file, long lastModified) {
System.out.println(file.isFile());
while (file.isDirectory()) {
System.out.println(file.i<caret>sFile());
System.out.println(file.length());
}
return file.lastModified();
}
}

View File

@@ -0,0 +1,13 @@
// "Replace with bulk 'Files.readAttributes' call" "true"
import java.io.*;
class Foo {
void printWhileDirectory(File file) {
System.out.println(file<caret>.isFile());
System.out.println(file.lastModified());
while (file.isDirectory()) {
System.out.println(file.isFile());
System.out.println(file.length());
}
}
}

View File

@@ -0,0 +1,15 @@
// "Replace with bulk 'Files.readAttributes' call" "true"
import java.io.*;
class Foo {
long isNewFile(File file, long lastModified) {
while (file.isDirectory()) {
System.out.println(file.isFile());
for (int i = 0; i < 10 && file.isDirectory(); i++) {
System.out.println(file.i<caret>sFile());
System.out.println(file.length());
}
}
return file.lastModified();
}
}

View File

@@ -0,0 +1,13 @@
// "Replace with bulk 'Files.readAttributes' call" "false"
import java.io.*;
class Foo {
long isNewFile(File file, long lastModified) {
System.out.println(file.isFile());
System.out.println(file.length());
while (file.isDirectory()) {
System.out.println(file.i<caret>sFile());
}
return file.lastModified();
}
}

View File

@@ -3,7 +3,7 @@ import java.io.*;
class Foo {
long isNewFile(File file) throws IOException {
while (file.isDire<caret>ctory()) {
if (file.isDirecto<caret>ry()) {
System.out.println(file.isFile());
}
return file.lastModified();

View File

@@ -3,7 +3,7 @@ import java.io.*;
class Foo {
long isNewFile(File file, long lastModified) {
while (file.isDire<caret>ctory()) {
if (file.isDirecto<caret>ry()) {
System.out.println(file.isFile());
}
return file.lastModified();