Skip to content

Commit

Permalink
SONARJAVA-4640 - Support javax packages migrated to jakarta (#4621)
Browse files Browse the repository at this point in the history
Update of SingleCharacterAlternationCheck, UnicodeCaseCheck, UnusedMethodParameterCheck,  UnusedPrivateMethodCheck, ServletMethodsExceptionsThrownCheck and TooManyParametersCheck
  • Loading branch information
ADarko22 authored Dec 20, 2023
1 parent ee671c9 commit 4f27457
Show file tree
Hide file tree
Showing 16 changed files with 175 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S1130",
"hasTruePositives": true,
"falseNegatives": 30,
"falseNegatives": 32,
"falsePositives": 0
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S1161",
"hasTruePositives": true,
"falseNegatives": 7,
"falseNegatives": 10,
"falsePositives": 0
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S1172",
"hasTruePositives": true,
"falseNegatives": 14,
"falseNegatives": 18,
"falsePositives": 0
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S1989",
"hasTruePositives": false,
"falseNegatives": 6,
"falseNegatives": 12,
"falsePositives": 0
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S5866",
"hasTruePositives": true,
"falseNegatives": 1,
"falseNegatives": 2,
"falsePositives": 0
}
12 changes: 9 additions & 3 deletions java-checks-test-sources/default/pom.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="http://maven.apache.org/POM/4.0.0"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>

<parent>
Expand Down Expand Up @@ -423,6 +423,12 @@
<version>6.0.0</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>jakarta.enterprise</groupId>
<artifactId>jakarta.enterprise.cdi-api</artifactId>
<version>4.0.1</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class ServletMethodsExceptionsThrownCheck extends HttpServlet {

private static boolean staticMethod() { return true; }

public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException {
String ip = request.getRemoteAddr();
InetAddress addr = InetAddress.getByName(ip); // Noncompliant [[sc=36;ec=45]] {{Handle the following exception that could be thrown by "getByName": UnknownHostException.}}
try {
Expand All @@ -32,7 +32,7 @@ public void foo(HttpServletRequest request, HttpServletResponse response) throws
InetAddress addr = InetAddress.getByName(ip);
}

public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
public void doPost(HttpServletRequest request, HttpServletResponse response) {
throw new IllegalStateException("bla"); // Noncompliant {{Handle the "IllegalStateException" thrown here in a "try/catch" block.}}
}
public void bar(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
Expand All @@ -47,3 +47,44 @@ protected void doPut(HttpServletRequest request, HttpServletResponse response) t
}
}
}


class JakartaServletMethodsExceptionsThrownCheck extends jakarta.servlet.http.HttpServlet {

private static boolean var = staticMethod();

private static boolean staticMethod() { return true; }

public void doGet(jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response) throws IOException {
String ip = request.getRemoteAddr();
InetAddress addr = InetAddress.getByName(ip); // Noncompliant [[sc=36;ec=45]] {{Handle the following exception that could be thrown by "getByName": UnknownHostException.}}
try {
addr = InetAddress.getByName(ip);
} catch (IllegalArgumentException e) {
throw e; // Noncompliant [[sc=7;ec=15]] {{Handle the "IllegalArgumentException" thrown here in a "try/catch" block.}}
} catch (Exception e) {
throw e; // Noncompliant {{Handle the "Exception" thrown here in a "try/catch" block.}}
}
staticMethod();
}

public void foo(jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response) throws IOException, jakarta.servlet.ServletException, NamingException {
String ip = request.getRemoteAddr();
InetAddress addr = InetAddress.getByName(ip);
}

public void doPost(jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response) {
throw new IllegalStateException("bla"); // Noncompliant {{Handle the "IllegalStateException" thrown here in a "try/catch" block.}}
}
public void bar(jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response) throws IOException, jakarta.servlet.ServletException {
throw new IllegalStateException("bla");
}

protected void doPut(jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response) throws jakarta.servlet.ServletException, IOException {
try {
foo(request, response); // Noncompliant [[sc=7;ec=10]] {{Handle the following exceptions that could be thrown by "foo": IOException, ServletException.}}
} catch (NamingException ne) {
throw new jakarta.servlet.ServletException(ne); // Noncompliant {{Handle the "ServletException" thrown here in a "try/catch" block.}}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import io.micronaut.http.annotation.Post;
import io.micronaut.http.annotation.Put;
import io.micronaut.http.annotation.Trace;
import java.lang.annotation.ElementType;
import java.lang.annotation.Target;

public class TooManyParametersCheck {
TooManyParametersCheck(int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8) { // Noncompliant {{Constructor has 8 parameters, which is greater than 7 authorized.}}
Expand All @@ -20,6 +22,9 @@ public class TooManyParametersCheck {
void otherMethod(int p1) {}

static void staticMethod(int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8) {} // Noncompliant

@CustomAnnotation
void customAnnotatedMethod(int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8) {} // Noncompliant
}

class TooManyParametersExtended extends TooManyParametersCheck {
Expand Down Expand Up @@ -81,4 +86,21 @@ public void trace(String p1, String p2, String p3, String p4, String p5, String
*/
record Record1(String p1, String p2, String p3, String p4, String p5, String p6, String p7, String p8) {} // Compliant


class JakartaMethodsUsingAnnotations {
@jakarta.ws.rs.GET
public void foo(String p1, String p2, String p3, String p4, String p5, String p6, String p7, String p8) {} // Compliant

@jakarta.ws.rs.POST
public void foo1(String p1, String p2, String p3, String p4, String p5, String p6, String p7, String p8) {} // Compliant

@jakarta.ws.rs.PUT
public void foo2(String p1, String p2, String p3, String p4, String p5, String p6, String p7, String p8) {} // Compliant

@jakarta.ws.rs.PATCH
public void foo3(String p1, String p2, String p3, String p4, String p5, String p6, String p7, String p8) {} // Compliant

@jakarta.inject.Inject
public void foo5(String p1, String p2, String p3, String p4, String p5, String p6, String p7, String p8) {} // Compliant
}
@Target(ElementType.METHOD)
@interface CustomAnnotation { }
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class UnusedPrivateMethodCheck {

private void init(@Observes Object object, String test) {} // Noncompliant
private void init(@javax.enterprise.event.Observes Object object) {} //Compliant, javax.enterprise.event.Observes is an exception to the rule
private void jakartaInit(@jakarta.enterprise.event.Observes Object object) {} //Compliant, jakarta.enterprise.event.Observes is an exception to the rule
private void initNc(@AnotherAnnotation Object object) {} // Noncompliant

private UnusedPrivateMethodCheck() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ public class SingleCharacterAlternationCheck {
@Pattern(regexp = "x|y|z") // Noncompliant [[sc=22;ec=27]] {{Replace this alternation with a character class.}}
String pattern;

@jakarta.validation.constraints.Pattern(regexp = "x|y|z") // Noncompliant [[sc=53;ec=58]] {{Replace this alternation with a character class.}}
String jakartaPattern;

void nonCompliant() {
String str = "abc123";
str.matches("a|b|c"); // Noncompliant [[sc=18;ec=23]] {{Replace this alternation with a character class.}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,16 @@ void compliant(String str) {
str.matches("(?iU)söme pättern");
str.matches("(?iU:söme) pättern");
}

@jakarta.validation.constraints.Email(
regexp = "söme pättern",
flags = jakarta.validation.constraints.Pattern.Flag.CASE_INSENSITIVE // Noncompliant [[sc=13;ec=73]] {{Also use "Flag.UNICODE_CASE" to correctly handle non-ASCII letters.}}
)
String jakartaEmail1;

@jakarta.validation.constraints.Email(
regexp = "söme pättern",
flags = { jakarta.validation.constraints.Pattern.Flag.CASE_INSENSITIVE, jakarta.validation.constraints.Pattern.Flag.UNICODE_CASE } // Compliant
)
String jakartaEmail2;
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
package checks.unused;

import java.util.function.Supplier;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.enterprise.event.Observes;
import java.io.IOException;
import java.io.NotSerializableException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.util.List;
import org.apache.struts.action.ActionMapping;
import org.apache.struts.action.ActionForm;
import java.util.function.Supplier;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.enterprise.event.Observes;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.struts.action.Action;
import org.apache.struts.action.ActionForm;
import org.apache.struts.action.ActionMapping;
import org.apache.struts.actions.BaseAction;

class UnusedMethodParameterCheck extends B {
Expand Down Expand Up @@ -346,3 +346,54 @@ private String bar(String a, String b) { // Compliant - used as method reference
return a;
}
}

class JakartaAnnotations {
void fooBar(int a, // Noncompliant [[sc=19;ec=20;secondary=+1,+3,+4]] {{Remove these unused method parameters "a", "b", "d", "e".}}
@jakarta.annotation.Nullable Boolean b,
int c,
int d,
@jakarta.annotation.Nullable Object e) {
System.out.println(c);
}
public void foo(@jakarta.enterprise.event.Observes Object event, int arg2) { // Compliant
System.out.println(arg2);
}

public void bar(@jakarta.annotation.Nonnull Object event, int arg2) { // Noncompliant {{Remove this unused method parameter "event".}} [[sc=54;ec=59]]
System.out.println(arg2);
}
}


class JakartaStrutsAction extends Action {
void foo(ActionMapping mapping, ActionForm form, jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response, String s) { // Compliant
System.out.println(s);
}

void bar(ActionMapping mapping, ActionForm form, jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response, String unused) { // Noncompliant {{Remove this unused method parameter "unused".}}
System.out.println("");
}

void qix(ActionMapping mapping, ActionForm form, jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response) { // Compliant
System.out.println("");
}
}

class JakartaStrutsAction2 extends BaseAction {
void foo(ActionMapping mapping, ActionForm form, jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response, String unused) { // Noncompliant {{Remove this unused method parameter "unused".}}
System.out.println("");
}

void bar(ActionMapping mapping, ActionForm form, jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response) { // Compliant
System.out.println("");
}
}

class JakartaNotStrutsAction {
void bar(ActionMapping mapping, ActionForm form, jakarta.servlet.http.HttpServletRequest request, jakarta.servlet.http.HttpServletResponse response) { // Noncompliant {{Remove this unused method parameter "response".}}
System.out.println(mapping);
System.out.println(form);
System.out.println(request);
System.out.println("");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@
public class ServletMethodsExceptionsThrownCheck extends IssuableSubscriptionVisitor {

private static final MethodMatchers IS_SERVLET_DO_METHOD = MethodMatchers.create()
.ofSubTypes("javax.servlet.http.HttpServlet").name(name -> name.startsWith("do")).withAnyParameters().build();
.ofSubTypes("javax.servlet.http.HttpServlet", "jakarta.servlet.http.HttpServlet")
.name(name -> name.startsWith("do"))
.withAnyParameters().build();

private final Deque<Boolean> shouldCheck = new ArrayDeque<>();
private final Deque<List<Type>> tryCatches = new ArrayDeque<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ public class TooManyParametersCheck extends IssuableSubscriptionVisitor {
"javax.ws.rs.PUT",
"javax.ws.rs.PATCH",
"javax.inject.Inject",
"jakarta.ws.rs.GET",
"jakarta.ws.rs.POST",
"jakarta.ws.rs.PUT",
"jakarta.ws.rs.PATCH",
"jakarta.inject.Inject",
"io.micronaut.http.annotation.Get",
"io.micronaut.http.annotation.Post",
"io.micronaut.http.annotation.Put",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,20 @@ public class UnusedMethodParameterCheck extends IssuableSubscriptionVisitor {
private static final String SECONDARY_MESSAGE_FORMAT = "Parameter \"%s\"";
private static final String QUICK_FIX_MESSAGE = "Remove \"%s\"";

private static final String AUTHORIZED_ANNOTATION = "javax.enterprise.event.Observes";
private static final Set<String> AUTHORIZED_ANNOTATIONS = Set.of("javax.enterprise.event.Observes", "jakarta.enterprise.event.Observes");
private static final String SUPPRESS_WARNINGS_ANNOTATION = "java.lang.SuppressWarnings";
private static final Set<String> EXCLUDED_WARNINGS_SUPPRESSIONS = SetUtils.immutableSetOf("\"rawtypes\"", "\"unchecked\"");
private static final MethodMatchers SERIALIZABLE_METHODS = MethodMatchers.or(
MethodMatchers.create().ofAnyType().names("writeObject").addParametersMatcher("java.io.ObjectOutputStream").build(),
MethodMatchers.create().ofAnyType().names("readObject").addParametersMatcher("java.io.ObjectInputStream").build());
private static final String STRUTS_ACTION_SUPERCLASS = "org.apache.struts.action.Action";
private static final Set<String> EXCLUDED_STRUTS_ACTION_PARAMETER_TYPES = SetUtils.immutableSetOf(
private static final Set<String> EXCLUDED_STRUTS_ACTION_PARAMETER_TYPES = Set.of(
"org.apache.struts.action.ActionMapping",
"org.apache.struts.action.ActionForm",
"javax.servlet.http.HttpServletRequest",
"javax.servlet.http.HttpServletResponse");
"javax.servlet.http.HttpServletResponse",
"jakarta.servlet.http.HttpServletRequest",
"jakarta.servlet.http.HttpServletResponse");

private static final UnresolvedIdentifiersVisitor UNRESOLVED_IDENTIFIERS_VISITOR = new UnresolvedIdentifiersVisitor();

Expand Down Expand Up @@ -111,7 +113,7 @@ public void visitNode(Tree tree) {

private static boolean isAnnotatedWithAuthorizedAnnotation(Symbol symbol) {
SymbolMetadata metadata = symbol.metadata();
return metadata.isAnnotatedWith(AUTHORIZED_ANNOTATION) || hasUnknownAnnotation(metadata);
return AUTHORIZED_ANNOTATIONS.stream().anyMatch(metadata::isAnnotatedWith) || hasUnknownAnnotation(metadata);
}

private void reportUnusedParameters(MethodTree methodTree, List<IdentifierTree> unused) {
Expand All @@ -124,7 +126,7 @@ private void reportUnusedParameters(MethodTree methodTree, List<IdentifierTree>
QuickFixHelper.newIssue(context)
.forRule(this)
.onTree(firstUnused)
.withMessage(unused.size() > 1 ? PRIMARY_PLURAL_MESSAGE_FORMAT : PRIMARY_SINGULAR_MESSAGE_FORMAT, parameterNames)
.withMessage(unused.size() > 1 ? PRIMARY_PLURAL_MESSAGE_FORMAT : PRIMARY_SINGULAR_MESSAGE_FORMAT, parameterNames)
.withSecondaries(secondaryLocations)
.withQuickFixes(() -> createQuickFixes(methodTree, unused))
.report();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ public class UnusedPrivateMethodCheck extends IssuableSubscriptionVisitor {
private final List<MethodTree> unusedPrivateMethods = new ArrayList<>();
private final Set<String> unresolvedMethodNames = new HashSet<>();

private static final String PARAM_ANNOTATION_EXCEPTION = "javax.enterprise.event.Observes";
private static final Set<String> PARAM_ANNOTATION_EXCEPTIONS = Set.of(
"javax.enterprise.event.Observes",
"jakarta.enterprise.event.Observes"
);

@Override
public List<Tree.Kind> nodesToVisit() {
Expand Down Expand Up @@ -180,7 +183,7 @@ private static boolean hasAllowedAnnotation(VariableTree variableTree) {

private static boolean isAllowedAnnotation(AnnotationTree annotation) {
Type annotationSymbolType = annotation.symbolType();
if (annotationSymbolType.is(PARAM_ANNOTATION_EXCEPTION)) {
if (PARAM_ANNOTATION_EXCEPTIONS.stream().anyMatch(annotationSymbolType::is)) {
return true;
}
if (annotationSymbolType.isUnknown()) {
Expand All @@ -189,7 +192,8 @@ private static boolean isAllowedAnnotation(AnnotationTree annotation) {
return "Observes".equals(((IdentifierTree) annotationType).name());
}
if (annotationType.is(Tree.Kind.MEMBER_SELECT)) {
return PARAM_ANNOTATION_EXCEPTION.equals(ExpressionsHelper.concatenate((MemberSelectExpressionTree) annotationType));
String concatenatedAnnotation = ExpressionsHelper.concatenate((MemberSelectExpressionTree) annotationType);
return PARAM_ANNOTATION_EXCEPTIONS.stream().anyMatch(concatenatedAnnotation::equals);
}
}
return false;
Expand Down

0 comments on commit 4f27457

Please sign in to comment.