17: Improve ExternalStoragePlugin
This commit is contained in:
@@ -86,8 +86,8 @@ public class PluginModule {
|
|||||||
|
|
||||||
@Provides
|
@Provides
|
||||||
@IntoSet
|
@IntoSet
|
||||||
public Plugin externalStoragePlugin(ParentNodeFinder parentNodeFinder) {
|
public Plugin externalStoragePlugin(ParentNodeFinder parentNodeFinder, StaticScopeHelper staticScopeHelper) {
|
||||||
return new ExternalStoragePlugin(parentNodeFinder);
|
return new ExternalStoragePlugin(parentNodeFinder, staticScopeHelper);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Provides
|
@Provides
|
||||||
|
|||||||
39
src/main/java/com/bartek/esa/core/helper/NodeUtil.java
Normal file
39
src/main/java/com/bartek/esa/core/helper/NodeUtil.java
Normal file
@@ -0,0 +1,39 @@
|
|||||||
|
package com.bartek.esa.core.helper;
|
||||||
|
|
||||||
|
import com.github.javaparser.Position;
|
||||||
|
import com.github.javaparser.ast.Node;
|
||||||
|
|
||||||
|
import static java.lang.String.format;
|
||||||
|
|
||||||
|
public class NodeUtil {
|
||||||
|
private Node first;
|
||||||
|
|
||||||
|
private NodeUtil(Node node) {
|
||||||
|
this.first = node;
|
||||||
|
}
|
||||||
|
|
||||||
|
public static NodeUtil is(Node node) {
|
||||||
|
return new NodeUtil(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
public boolean after(Node second) {
|
||||||
|
Position firstPosition = getPosition(first);
|
||||||
|
Position secondPosition = getPosition(second);
|
||||||
|
return firstPosition.isAfter(secondPosition);
|
||||||
|
}
|
||||||
|
|
||||||
|
private Position getPosition(Node node) {
|
||||||
|
return node.getRange()
|
||||||
|
.map(r -> r.begin)
|
||||||
|
.orElseGet(() -> {
|
||||||
|
System.err.println(format("Cannot determine position of:\n%s\nProduced results might not be reliable."));
|
||||||
|
return new Position(0, 0);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
public boolean before(Node second) {
|
||||||
|
Position firstPosition = getPosition(first);
|
||||||
|
Position secondPosition = getPosition(second);
|
||||||
|
return firstPosition.isBefore(secondPosition);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -20,6 +20,12 @@ public class StaticScopeHelper {
|
|||||||
|
|
||||||
public Predicate<MethodCallExpr> isFromScope(CompilationUnit compilationUnit, String methodName, String scope, String importScope) {
|
public Predicate<MethodCallExpr> isFromScope(CompilationUnit compilationUnit, String methodName, String scope, String importScope) {
|
||||||
return expr -> {
|
return expr -> {
|
||||||
|
if(!expr.getName().getIdentifier().matches(methodName)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// is called with scope
|
||||||
|
// Class.method()
|
||||||
boolean isFromScope = expr.getScope()
|
boolean isFromScope = expr.getScope()
|
||||||
.filter(Expression::isNameExpr)
|
.filter(Expression::isNameExpr)
|
||||||
.map(Expression::asNameExpr)
|
.map(Expression::asNameExpr)
|
||||||
@@ -28,6 +34,9 @@ public class StaticScopeHelper {
|
|||||||
.map(s -> s.equals(scope))
|
.map(s -> s.equals(scope))
|
||||||
.orElse(false);
|
.orElse(false);
|
||||||
|
|
||||||
|
// is from static import
|
||||||
|
// import static a.b.Class.method
|
||||||
|
// method()
|
||||||
if(!isFromScope) {
|
if(!isFromScope) {
|
||||||
isFromScope = compilationUnit.findAll(ImportDeclaration.class).stream()
|
isFromScope = compilationUnit.findAll(ImportDeclaration.class).stream()
|
||||||
.filter(ImportDeclaration::isStatic)
|
.filter(ImportDeclaration::isStatic)
|
||||||
@@ -39,6 +48,9 @@ public class StaticScopeHelper {
|
|||||||
.anyMatch(q -> q.equals(format("%s.%s", importScope, scope)));
|
.anyMatch(q -> q.equals(format("%s.%s", importScope, scope)));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// is from import
|
||||||
|
// import static a.b.Class.*
|
||||||
|
// method()
|
||||||
if(!isFromScope) {
|
if(!isFromScope) {
|
||||||
isFromScope = compilationUnit.findAll(ImportDeclaration.class).stream()
|
isFromScope = compilationUnit.findAll(ImportDeclaration.class).stream()
|
||||||
.filter(ImportDeclaration::isStatic)
|
.filter(ImportDeclaration::isStatic)
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package com.bartek.esa.core.plugin;
|
|||||||
import com.bartek.esa.context.model.Source;
|
import com.bartek.esa.context.model.Source;
|
||||||
import com.bartek.esa.core.archetype.JavaPlugin;
|
import com.bartek.esa.core.archetype.JavaPlugin;
|
||||||
import com.bartek.esa.core.helper.ParentNodeFinder;
|
import com.bartek.esa.core.helper.ParentNodeFinder;
|
||||||
|
import com.bartek.esa.core.helper.StaticScopeHelper;
|
||||||
import com.bartek.esa.core.model.enumeration.Severity;
|
import com.bartek.esa.core.model.enumeration.Severity;
|
||||||
import com.github.javaparser.ast.CompilationUnit;
|
import com.github.javaparser.ast.CompilationUnit;
|
||||||
import com.github.javaparser.ast.body.MethodDeclaration;
|
import com.github.javaparser.ast.body.MethodDeclaration;
|
||||||
@@ -11,18 +12,22 @@ import com.github.javaparser.ast.expr.MethodCallExpr;
|
|||||||
import javax.inject.Inject;
|
import javax.inject.Inject;
|
||||||
import java.util.function.Consumer;
|
import java.util.function.Consumer;
|
||||||
|
|
||||||
|
import static com.bartek.esa.core.helper.NodeUtil.is;
|
||||||
|
|
||||||
public class ExternalStoragePlugin extends JavaPlugin {
|
public class ExternalStoragePlugin extends JavaPlugin {
|
||||||
private final ParentNodeFinder parentNodeFinder;
|
private final ParentNodeFinder parentNodeFinder;
|
||||||
|
private final StaticScopeHelper staticScopeHelper;
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
public ExternalStoragePlugin(ParentNodeFinder parentNodeFinder) {
|
public ExternalStoragePlugin(ParentNodeFinder parentNodeFinder, StaticScopeHelper staticScopeHelper) {
|
||||||
this.parentNodeFinder = parentNodeFinder;
|
this.parentNodeFinder = parentNodeFinder;
|
||||||
|
this.staticScopeHelper = staticScopeHelper;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void run(Source<CompilationUnit> java) {
|
public void run(Source<CompilationUnit> java) {
|
||||||
java.getModel().findAll(MethodCallExpr.class).stream()
|
java.getModel().findAll(MethodCallExpr.class).stream()
|
||||||
.filter(expr -> expr.getName().getIdentifier().matches("getExternalStorageDirectory|getExternalStoragePublicDirectory"))
|
.filter(staticScopeHelper.isFromScope(java.getModel(), "getExternalStorageDirectory|getExternalStoragePublicDirectory", "Environment", "android.os"))
|
||||||
.forEach(findCheckingStorageStateForAccessingExternalStorage(java));
|
.forEach(findCheckingStorageStateForAccessingExternalStorage(java));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -36,7 +41,8 @@ public class ExternalStoragePlugin extends JavaPlugin {
|
|||||||
|
|
||||||
private void findCheckingStorageStateInMethodDeclaration(Source<CompilationUnit> java, MethodCallExpr accessingToExternalStorage, MethodDeclaration methodDeclaration) {
|
private void findCheckingStorageStateInMethodDeclaration(Source<CompilationUnit> java, MethodCallExpr accessingToExternalStorage, MethodDeclaration methodDeclaration) {
|
||||||
boolean isStateBeingChecked = methodDeclaration.findAll(MethodCallExpr.class).stream()
|
boolean isStateBeingChecked = methodDeclaration.findAll(MethodCallExpr.class).stream()
|
||||||
.anyMatch(e -> e.getName().getIdentifier().equals("getExternalStorageState"));
|
.filter(staticScopeHelper.isFromScope(java.getModel(), "getExternalStorageState", "Environment", "android.os"))
|
||||||
|
.anyMatch(checkingMethod -> is(accessingToExternalStorage).after(checkingMethod));
|
||||||
|
|
||||||
if (!isStateBeingChecked) {
|
if (!isStateBeingChecked) {
|
||||||
addJavaIssue(Severity.WARNING, java.getFile(), accessingToExternalStorage);
|
addJavaIssue(Severity.WARNING, java.getFile(), accessingToExternalStorage);
|
||||||
|
|||||||
Reference in New Issue
Block a user