Skip to content

Commit

Permalink
Introduce additional arguments to RequestParamType
Browse files Browse the repository at this point in the history
  • Loading branch information
@chamil-prabodha
chamil-prabodha committed Dec 28, 2022
1 parent b220786 commit 5f70f62
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 7 deletions.
59 changes: 52 additions & 7 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RequestParamType.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package tech.picnic.errorprone.bugpatterns;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
Expand All @@ -9,17 +10,21 @@
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
import static com.google.errorprone.matchers.Matchers.isType;
import static com.google.errorprone.matchers.Matchers.not;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;

import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;

/** A {@link BugChecker} that flags {@code @RequestParam} parameters with an unsupported type. */
Expand All @@ -32,18 +37,58 @@
tags = LIKELY_ERROR)
public final class RequestParamType extends BugChecker implements VariableTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<VariableTree> HAS_UNSUPPORTED_REQUEST_PARAM =
allOf(
annotations(AT_LEAST_ONE, isType("org.springframework.web.bind.annotation.RequestParam")),
anyOf(isSubtypeOf(ImmutableCollection.class), isSubtypeOf(ImmutableMap.class)));
private static final String FLAG_PREFIX = "RequestParamType:";
private static final String INCLUDED_CLASS_FLAG = FLAG_PREFIX + "Includes";

/** Instantiates a new {@link RequestParamType} instance. */
public RequestParamType() {}
private final Matcher<VariableTree> hasUnsupportedRequestParams;

/** Instantiates a default {@link RequestParamType} instance. */
public RequestParamType() {
this(ErrorProneFlags.empty());
}

/**
* Instantiates a customized {@link RequestParamType} instance.
*
* @param flags Any provided command line flags.
*/
public RequestParamType(ErrorProneFlags flags) {
hasUnsupportedRequestParams = createVariableTreeMatcher(flags);
}

@Override
public Description matchVariable(VariableTree tree, VisitorState state) {
return HAS_UNSUPPORTED_REQUEST_PARAM.matches(tree, state)
return hasUnsupportedRequestParams.matches(tree, state)
? describeMatch(tree)
: Description.NO_MATCH;
}

private static Matcher<VariableTree> createVariableTreeMatcher(ErrorProneFlags flags) {
return allOf(
annotations(AT_LEAST_ONE, isType("org.springframework.web.bind.annotation.RequestParam")),
anyOf(isSubtypeOf(ImmutableCollection.class), isSubtypeOf(ImmutableMap.class)),
not(anyOf(getSupportedClasses(includedClassNames(flags)))));
}

private static ImmutableList<String> includedClassNames(ErrorProneFlags flags) {
return flags.getList(INCLUDED_CLASS_FLAG).map(ImmutableList::copyOf).orElse(ImmutableList.of());
}

private static ImmutableList<Matcher<Tree>> getSupportedClasses(
ImmutableList<String> inclusions) {
return inclusions.stream()
.filter(inclusion -> !inclusion.isEmpty())
.map(String::trim)
.map(inclusion -> isSubtypeOf(createClass(inclusion)))
.collect(toImmutableList());
}

private static Class<?> createClass(String className) {
try {
return Class.forName(className);
} catch (ClassNotFoundException e) {
throw new IllegalArgumentException(
String.format("Invalid class name '%s' in `%s`", className, INCLUDED_CLASS_FLAG), e);
}
}
}
104 changes: 104 additions & 0 deletions ...-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestParamTypeTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
package tech.picnic.errorprone.bugpatterns;

import com.google.common.collect.ImmutableList;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;

final class RequestParamTypeTest {
private final CompilationTestHelper compilationTestHelper =
CompilationTestHelper.newInstance(RequestParamType.class, getClass());
private final CompilationTestHelper restrictedCompilationTestHelper =
CompilationTestHelper.newInstance(RequestParamType.class, getClass())
.setArgs(
ImmutableList.of(
"-XepOpt:RequestParamType:Includes=com.google.common.collect.ImmutableCollection"));
private final CompilationTestHelper restrictedWithSubTypeCompilationTestHelper =
CompilationTestHelper.newInstance(RequestParamType.class, getClass())
.setArgs(
ImmutableList.of(
"-XepOpt:RequestParamType:Includes=com.google.common.collect.ImmutableSet"));

@Test
void identification() {
Expand Down Expand Up @@ -63,4 +74,97 @@ void identification() {
"}")
.doTest();
}

@Test
void identificationOfIncludedClass() {
restrictedCompilationTestHelper
.addSourceLines(
"A.java",
"import com.google.common.collect.ImmutableBiMap;",
"import com.google.common.collect.ImmutableList;",
"import com.google.common.collect.ImmutableMap;",
"import com.google.common.collect.ImmutableSet;",
"import java.util.List;",
"import java.util.Map;",
"import java.util.Set;",
"import org.jspecify.annotations.Nullable;",
"import org.springframework.web.bind.annotation.DeleteMapping;",
"import org.springframework.web.bind.annotation.GetMapping;",
"import org.springframework.web.bind.annotation.PostMapping;",
"import org.springframework.web.bind.annotation.PutMapping;",
"import org.springframework.web.bind.annotation.RequestBody;",
"import org.springframework.web.bind.annotation.RequestParam;",
"",
"interface A {",
" @PostMapping",
" A properRequestParam(@RequestBody String body);",
"",
" @GetMapping",
" A properRequestParam(@RequestParam int param);",
"",
" @GetMapping",
" A properRequestParam(@RequestParam List<String> param);",
"",
" @PostMapping",
" A properRequestParam(@RequestBody String body, @RequestParam Set<String> param);",
"",
" @PutMapping",
" A properRequestParam(@RequestBody String body, @RequestParam Map<String, String> param);",
"",
" @GetMapping",
" // BUG: Diagnostic contains:",
" A get(@RequestParam ImmutableBiMap<String, String> param);",
"",
" @PostMapping",
" A post(@Nullable @RequestParam ImmutableList<String> param);",
"",
" @PutMapping",
" A put(@RequestBody String body, @RequestParam ImmutableSet<String> param);",
"",
" @DeleteMapping",
" // BUG: Diagnostic contains:",
" A delete(@RequestBody String body, @RequestParam ImmutableMap<String, String> param);",
"",
" void negative(ImmutableSet<Integer> set, ImmutableMap<String, String> map);",
"}")
.doTest();
}

@Test
void identificationOfIncludedSubClass() {
restrictedWithSubTypeCompilationTestHelper
.addSourceLines(
"A.java",
"import com.google.common.collect.ImmutableBiMap;",
"import com.google.common.collect.ImmutableList;",
"import com.google.common.collect.ImmutableMap;",
"import com.google.common.collect.ImmutableSet;",
"import org.jspecify.annotations.Nullable;",
"import org.springframework.web.bind.annotation.DeleteMapping;",
"import org.springframework.web.bind.annotation.GetMapping;",
"import org.springframework.web.bind.annotation.PostMapping;",
"import org.springframework.web.bind.annotation.PutMapping;",
"import org.springframework.web.bind.annotation.RequestBody;",
"import org.springframework.web.bind.annotation.RequestParam;",
"",
"interface A {",
" @GetMapping",
" // BUG: Diagnostic contains:",
" A get(@RequestParam ImmutableBiMap<String, String> param);",
"",
" @PostMapping",
" // BUG: Diagnostic contains:",
" A post(@Nullable @RequestParam ImmutableList<String> param);",
"",
" @PutMapping",
" A put(@RequestBody String body, @RequestParam ImmutableSet<String> param);",
"",
" @DeleteMapping",
" // BUG: Diagnostic contains:",
" A delete(@RequestBody String body, @RequestParam ImmutableMap<String, String> param);",
"",
" void negative(ImmutableSet<Integer> set, ImmutableMap<String, String> map);",
"}")
.doTest();
}
}

0 comments on commit 5f70f62

Please sign in to comment.