Skip to content

Commit

Permalink
SONARJAVA-4951 Replace sonar-java specific verifier with analyzer-com…
Browse files Browse the repository at this point in the history
…mons library (#4786)

Use the CheckVerifier provided by the sonar-analyzer-commons testing module instead of the sonar-java-specific implementation to improve testing and allow for better maintanance and updates of the verifier logic.
  • Loading branch information
irina-batinic-sonarsource committed May 13, 2024
1 parent 308fcf3 commit f74679d
Show file tree
Hide file tree
Showing 899 changed files with 8,164 additions and 4,648 deletions.
19 changes: 15 additions & 4 deletions docs/CUSTOM_RULES_101.md
Original file line number Diff line number Diff line change
Expand Up @@ -682,14 +682,25 @@ See: [pom.xml#L137-L197](./java-custom-rules-example/pom_SQ_9_9_LTS.xml#L137-L19
You can raise an issue on a given line, but you can also raise it at a specific Token.
Because of that, you may want to specify, in your sample code used by your Unit Tests, the exact location, i.e. in between which 2 specific Columns, where you are expecting the issue to be raised.

This can be achieved using the special keywords `sc` (start-column) and `ec` (end-column) in the `// Noncompliant` comment.
In the following example, we are expecting to have the issue being raised between the columns 27 and 32 (i.e. exactly on the "Order" variable type):

> **Deprecated**
>
>This can be achieved using the special keywords `sc` (start-column) and `ec` (end-column) in the `// Noncompliant` comment.
>In the following example, we are expecting to have the issue being raised between the columns 27 and 32 (i.e. exactly on the "Order" variable type):
> ```java
> public String updateOrder(Order order){ // Noncompliant [[sc=27;ec=32]] {{Don't use Order here because it's an @Entity}}
> // ...
> }
> ```
Assert precise primary location of an issue with the comment on a line below containing required number of `^`.
```java
public String updateOrder(Order order){ // Noncompliant [[sc=27;ec=32]] {{Don't use Order here because it's an @Entity}}
public String updateOrder(Order order){ // Noncompliant {{Don't use Order here because it's an @Entity}}
// ^^^^^
// ...
}
```
More information on how to test precise issue location can be found in: [SonarSource Analyzer Test Commons](https://github.com/SonarSource/sonar-analyzer-commons/tree/master/test-commons) library.


### How to test the Source Version in a rule

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ public class ATest {

@Test
public void myTest() {
if (someCondition) { // Noncompliant [[sc=5;ec=7]] {{Remove this 'if' statement from this test.}}
if (someCondition) { // Noncompliant {{Remove this 'if' statement from this test.}}
// ^^
// verify something
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ class BusinessClassDelegate implements MySecurityInterface, SecondInterface {
public void aMethod() { }

@org.foo.MyOtherSecurityAnnotation
public void anotherMethod() { } // Noncompliant [[sc=15;ec=28]] {{Mandatory Annotation not set @MySecurityAnnotation}}
public void anotherMethod() { } // Noncompliant {{Mandatory Annotation not set @MySecurityAnnotation}}
// ^^^^^^^^^^^^^

@VariousAnnotation
public void differentMethod() { } // Noncompliant [[startColumn=15;endColumn=30]] {{Mandatory Annotation not set @MySecurityAnnotation}}
public void differentMethod() { } // Noncompliant {{Mandatory Annotation not set @MySecurityAnnotation}}
// ^^^^^^^^^^^^^^^
}

class OtherClass implements FirstInterface, SecondInterface {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
public class HelloController {

@RequestMapping("/updateOrder")
public String updateOrder(Order order, Client client) { // Noncompliant [[sc=29;ec=34]] {{Don't use Order here because it's an @Entity}}
public String updateOrder(Order order, Client client) { // Noncompliant {{Don't use Order here because it's an @Entity}}
// ^^^^^
return null;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S1128",
"hasTruePositives": true,
"falseNegatives": 34,
"falseNegatives": 33,
"falsePositives": 0
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S2699",
"hasTruePositives": true,
"falseNegatives": 151,
"falseNegatives": 150,
"falsePositives": 1
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S5786",
"hasTruePositives": true,
"falseNegatives": 63,
"falseNegatives": 62,
"falsePositives": 0
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ public class AwsConsumerBuilderUsageCheckSample {

public void test() {
AwsConsumerBuilderUsageCheckSample.builder()
.validDestination(Destination.builder() // Noncompliant [[sc=8;ec=24]] {{Consider using the Consumer Builder method instead of creating this nested builder.}}
.validDestination(Destination.builder() // Noncompliant {{Consider using the Consumer Builder method instead of creating this nested builder.}}
// ^^^^^^^^^^^^^^^^
.toAddresses("to-email@domain.com")
.bccAddresses("bcc-email@domain.com")
.build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ public class AwsConsumerBuilderUsageCheckSample {
void test_destination(Destination destination1) {

software.amazon.awssdk.services.sesv2.model.SendEmailRequest.builder()
.destination(Destination.builder() // Noncompliant [[sc=8;ec=19]] {{Consider using the Consumer Builder method instead of creating this nested builder.}}
.destination(Destination.builder() // Noncompliant {{Consider using the Consumer Builder method instead of creating this nested builder.}}
// ^^^^^^^^^^^
.toAddresses("to-email@domain.com")
.bccAddresses("bcc-email@domain.com")
.build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,28 @@ public class AwsCredentialsShouldBeSetExplicitlyCheckSample {
public static final AwsClientBuilder BUILDER = S3Client.builder(); // Compliant FN - Could be configured closer to build call

void nonCompliant() {
S3Client.builder().build(); // Noncompliant [[sc=5;ec=31]] {{Set the credentials provider explicitly on this builder.}}
S3Client client = S3Client.builder().build(); // Noncompliant [[sc=23;ec=49]] {{Set the credentials provider explicitly on this builder.}}
S3Client otherClient = S3Client.builder() // Noncompliant [[sl=+0;sc=28;el=+2;ec=15]] {{Set the credentials provider explicitly on this builder.}}
S3Client.builder().build(); // Noncompliant {{Set the credentials provider explicitly on this builder.}}
// ^^^^^^^^^^^^^^^^^^^^^^^^^^
S3Client client = S3Client.builder().build(); // Noncompliant {{Set the credentials provider explicitly on this builder.}}
// ^^^^^^^^^^^^^^^^^^^^^^^^^^
S3Client otherClient = S3Client.builder() // Noncompliant {{Set the credentials provider explicitly on this builder.}}
//^[sc=28;ec=14;el=+3]
.region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
.build();
}

void nonCompliantBuilderAsVariable() {
S3ClientBuilder builder = S3Client.builder(); // Noncompliant [[sc=5;ec=50]] {{Set the credentials provider explicitly on this builder.}}
S3ClientBuilder builder = S3Client.builder(); // Noncompliant {{Set the credentials provider explicitly on this builder.}}
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
builder.build();

S3ClientBuilder secondBuilder = S3Client.builder(); // Noncompliant [[sc=5;ec=56]] {{Set the credentials provider explicitly on this builder.}}
S3ClientBuilder secondBuilder = S3Client.builder(); // Noncompliant {{Set the credentials provider explicitly on this builder.}}
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
secondBuilder.region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())));
secondBuilder.build();

S3ClientBuilder thirdBuilder = S3Client.builder() // Noncompliant [[sl=+0;sc=5;el=+1;ec=92]] {{Set the credentials provider explicitly on this builder.}}
S3ClientBuilder thirdBuilder = S3Client.builder() // Noncompliant {{Set the credentials provider explicitly on this builder.}}
//^[sc=5;ec=91;el=+2]
.region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())));
thirdBuilder.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public Void handleRequest(Object o, Context context) {
.withCredentials(new ProfileCredentialsProvider())
.withRegion(Regions.US_WEST_2).build();

awsLambda.invoke(invokeRequest); // Noncompliant [[sc=17;ec=23]] {{Avoid synchronous calls to other lambdas.}}
awsLambda.invoke(invokeRequest); // Noncompliant {{Avoid synchronous calls to other lambdas.}}
// ^^^^^^

invokeSync();
invokeAsync();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

public class AwsLongTermAccessKeysCheckSample {
void noncompliant() {
BasicAWSCredentials foo = new BasicAWSCredentials("", ""); // Noncompliant [[sc=35;ec=54]] {{Make sure using a long-term access key is safe here.}}
BasicAWSCredentials foo = new BasicAWSCredentials("", ""); // Noncompliant {{Make sure using a long-term access key is safe here.}}
// ^^^^^^^^^^^^^^^^^^^
}

void compliant() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ public class AwsRegionSetterCheckSample {
private static final Regions ENUM_EU_WEST_1 = Regions.EU_WEST_1;

void nonCompliant() {
AmazonS3ClientBuilder.standard().withRegion("eu_west_1").build(); // Noncompliant [[sc=49;ec=60]]
AmazonS3ClientBuilder.standard().withRegion("eu_west_1").build(); // Noncompliant
// ^^^^^^^^^^^
AWSLambdaClientBuilder.standard().setRegion("eu_west_1"); // Noncompliant {{Give the enum value for this region instead.}}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,21 @@ public class AwsRegionShouldBeSetExplicitlyCheckSample {
public static final AwsClientBuilder BUILDER = getABuilder();

void nonCompliantChained() {
S3Client s3Client = S3Client.builder() // Noncompliant [[sl=+0;sc=25;el=+2;ec=15]] {{Set the region explicitly on this builder.}}
S3Client s3Client = S3Client.builder() // Noncompliant {{Set the region explicitly on this builder.}}
//^[el=+3;ec=14]
.credentialsProvider(EnvironmentVariableCredentialsProvider.create())
.build();
}

void nonCompliantVariable() {
S3ClientBuilder builder = S3Client.builder(); // Noncompliant [[sc=5;ec=50]] {{Set the region explicitly on this builder.}}
S3ClientBuilder builder = S3Client.builder(); // Noncompliant {{Set the region explicitly on this builder.}}
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
builder.build();
}

class NonCompliantClientAsField {
S3Client incompleteClient = S3Client.builder().build(); // Noncompliant [[sc=33;ec=59]] {{Set the region explicitly on this builder.}}
S3Client incompleteClient = S3Client.builder().build(); // Noncompliant {{Set the region explicitly on this builder.}}
// ^^^^^^^^^^^^^^^^^^^^^^^^^^
}

void compliantChained() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@ public RequestHandlerImpl() throws SQLException {
@SneakyThrows
@Override
public Void handleRequest(Object o, Context context) {
S3Client s3Client = S3Client.builder().region(Region.EU_CENTRAL_1).build(); // Noncompliant [[sc=74;ec=79]] {Instantiate this client outside the Lambda function.}
S3Client.builder().build(); // Noncompliant [[sc=26;ec=31]] {Instantiate this client outside the Lambda function.}
MachineLearningClient.builder().build(); // Noncompliant [[sc=39;ec=44]] {Instantiate this client outside the Lambda function.}
DriverManager.getConnection("foo"); // Noncompliant [[sc=21;ec=34]] {Instantiate this client outside the Lambda function.}
var customClient = new FooClient(); // Noncompliant [[sc=30;ec=39]] {Instantiate this client outside the Lambda function.}
S3Client s3Client = S3Client.builder().region(Region.EU_CENTRAL_1).build(); // Noncompliant {{Instantiate this client outside the Lambda function.}}
// ^^^^^
S3Client.builder().build(); // Noncompliant {{Instantiate this client outside the Lambda function.}}
// ^^^^^
MachineLearningClient.builder().build(); // Noncompliant {{Instantiate this client outside the Lambda function.}}
// ^^^^^
DriverManager.getConnection("foo"); // Noncompliant {{Instantiate this database connection outside the Lambda function.}}
// ^^^^^^^^^^^^^
var customClient = new FooClient(); // Noncompliant {{Instantiate this client outside the Lambda function.}}
// ^^^^^^^^^
var compliant = new Object();
build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,25 @@

public class HardCodedCredentialsShouldNotBeUsedCheckSample {
static final String FINAL_SECRET_STRING = "hunter2";
// ^^^^^^^^^>
static final byte[] FINAL_SECRET_BYTE_ARRAY = FINAL_SECRET_STRING.getBytes(StandardCharsets.UTF_8);
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^>
private static String secretStringField = "hunter2";
private static byte[] secretByteArrayField = new byte[]{0xC, 0xA, 0xF, 0xE};
private static char[] secretCharArrayField = new char[]{0xC, 0xA, 0xF, 0xE};
private static CharSequence secretCharSequenceField = "Hello, World!".subSequence(0, 12);

public static void nonCompliant(byte[] message, boolean condition, Charset encoding, SignatureAlgorithm paremSignatureAlgorithm) throws ServletException, KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException, UnsupportedEncodingException, jakarta.servlet.ServletException {
// byte array based
SHA256.getHMAC(FINAL_SECRET_BYTE_ARRAY, message); // Noncompliant {{Revoke and change this password, as it is compromised.}}
// ^^^^^^^^^^^^^^^^^^^^^^^

String effectivelyConstantString = "s3cr37";
// ^^^^^^^^>
byte[] key = effectivelyConstantString.getBytes();

// byte array based
SHA256.getHMAC(FINAL_SECRET_BYTE_ARRAY, message); // Noncompliant [[sc=20;ec=43;secondary=-11,-12]] {{Revoke and change this password, as it is compromised.}}
SHA256.getHMAC(key, message); // Noncompliant [[sc=20;ec=23;secondary=-4,-5]]
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^>
SHA256.getHMAC(key, message); // Noncompliant
// ^^^
SHA256.getHMAC(effectivelyConstantString.getBytes(), message); // Noncompliant
SHA256.getHMAC("anotherS3cr37".getBytes(), message); // Noncompliant
SHA256.getHMAC(FINAL_SECRET_STRING.getBytes(), message); // Noncompliant
Expand All @@ -56,18 +62,36 @@ public static void nonCompliant(byte[] message, boolean condition, Charset encod
// String based
HttpServletRequest request = new HttpServletRequestWrapper(null);
request.login("user", "password"); // Noncompliant
request.login("user", effectivelyConstantString); // Noncompliant [[sc=27;ec=52;secondary=-17]]
request.login("user", FINAL_SECRET_STRING); // Noncompliant [[sc=27;ec=46;secondary=-26]]
request.login("user", effectivelyConstantString); // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^^^^^^
request.login("user", FINAL_SECRET_STRING); // Noncompliant
// ^^^^^^^^^^^^^^^^^^^
String plainTextSecret = new String("BOOM");
// ^^^^^^^^^^^^^^^^^^>
request.login("user", plainTextSecret); // Noncompliant
// ^^^^^^^^^^^^^^^
request.login("user", new String("secret")); // Noncompliant
request.login("user", new String(FINAL_SECRET_BYTE_ARRAY, 0, 7)); // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^@36<
// ^^^^^^^^^@34<
request.login("user", new String(FINAL_SECRET_BYTE_ARRAY, encoding)); // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^@36<
// ^^^^^^^^^@34<

String conditionalButPredictable = condition ? FINAL_SECRET_STRING : plainTextSecret;
request.login("user", conditionalButPredictable); // Noncompliant [[sc=27;ec=52;secondary=-33,-6,-1]]
request.login("user", Json.MEDIA_TYPE); // Noncompliant [[sc=27;ec=42]]
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^>
request.login("user", conditionalButPredictable); // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^^^^^^
// ^^^^^^^^^@34<
// ^^^^^^^^^^^^^^^^^^@69<
request.login("user", Json.MEDIA_TYPE); // Noncompliant
// ^^^^^^^^^^^^^^^
String concatenatedPassword = "abc" + true + ":" + 12 + ":" + 43L + ":" + 'a' + ":" + 0.2f + ":" + 0.2d;
request.login("user", concatenatedPassword); // Noncompliant [[sc=27;ec=47;secondary=-1]]
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^>
request.login("user", concatenatedPassword); // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^

jakarta.servlet.http.HttpServletRequest requestJakarta = new jakarta.servlet.http.HttpServletRequestWrapper(null);
requestJakarta.login("user", "password"); // Noncompliant
Expand All @@ -76,17 +100,23 @@ public static void nonCompliant(byte[] message, boolean condition, Charset encod
store.getKey("", new char[]{0xC, 0xA, 0xF, 0xE}); // Noncompliant

char[] password = new char[]{0xC, 0xA, 0xF, 0xE};
store.getKey("", password); // Noncompliant [[sc=22;ec=30;secondary=-1]]
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^>
store.getKey("", password); // Noncompliant
// ^^^^^^^^

String passwordAsString = "hunter2";
store.getKey("", passwordAsString.toCharArray()); // Noncompliant [[sc=22;ec=52]]
store.getKey("", passwordAsString.toCharArray()); // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

char[] reassignedArray;
// ^^^^^^^^^^^^^^^^^^^^^^^>
reassignedArray = new char[]{'a', 'b', 'c', 'd', 'e', 'f'};
reassignedArray = new char[]{'a', 'b', 'c', 'd', 'e', 'f'};
store.getKey("", reassignedArray); // Noncompliant [[sc=22;ec=37;secondary=-3]]
store.getKey("", reassignedArray); // Noncompliant
// ^^^^^^^^^^^^^^^

Encryptors.delux(effectivelyConstantString.subSequence(0, effectivelyConstantString.length()), effectivelyConstantString); // Noncompliant [[sc=22;ec=98]]
Encryptors.delux(effectivelyConstantString.subSequence(0, effectivelyConstantString.length()), effectivelyConstantString); // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Encryptors.delux("password".subSequence(0, 0), "salt"); // Noncompliant

new Pbkdf2PasswordEncoder("secret"); // Noncompliant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ abstract class AbstractClassNoFieldShouldBeInterfaceCheckA {

abstract void method();
}
abstract class AbstractClassNoFieldShouldBeInterfaceCheckB { // Noncompliant [[sc=16;ec=59]] {{Convert the abstract class "AbstractClassNoFieldShouldBeInterfaceCheckB" into an interface.}}
abstract class AbstractClassNoFieldShouldBeInterfaceCheckB { // Noncompliant {{Convert the abstract class "AbstractClassNoFieldShouldBeInterfaceCheckB" into an interface.}}
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
int method(){
return 1;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ abstract class A {

abstract void method();
}
abstract class B { // Noncompliant [[sc=16;ec=17]] {{Convert the abstract class "B" into an interface.}}
abstract class B { // Noncompliant {{Convert the abstract class "B" into an interface.}}
// ^
int method(){
return 1;
}
Expand Down

0 comments on commit f74679d

Please sign in to comment.