Skip to content

Commit

Permalink
Merge pull request #70 from fermadeiral/review-tests
Browse files Browse the repository at this point in the history
  • Loading branch information
monperrus authored Apr 3, 2020
2 parents 3d6dbad + 4bb6cde commit 44fca83
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 42 deletions.
11 changes: 11 additions & 0 deletions docs/HANDLED_SONARQUBE_RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,17 @@ Example:
+ BigDecimal bd3 = BigDecimal.valueOf(f);
```

When the constructor of `BigDecimal` being called has two arguments, being the first one of type `float` or `double`, that argument is changed to `String`.

Example:
```diff
MathContext mc;
- BigDecimal bd4 = new BigDecimal(2.0, mc); // Noncompliant {{Use "BigDecimal.valueOf" instead.}}
- BigDecimal bd6 = new BigDecimal(2.0f, mc); // Noncompliant {{Use "BigDecimal.valueOf" instead.}}
+ BigDecimal bd4 = new BigDecimal("2.0", mc);
+ BigDecimal bd6 = new BigDecimal("2.0", mc);
```

Check out an accepted PR in [Apache PDFBox](https://github.com/apache/pdfbox/pull/76) that repairs one BigDecimalDoubleConstructor violation.

-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import spoon.reflect.code.CtConstructorCall;
import spoon.reflect.code.CtExpression;
import spoon.reflect.code.CtInvocation;
import spoon.reflect.code.CtLiteral;
import spoon.reflect.declaration.CtMethod;
import spoon.reflect.declaration.CtType;
import spoon.reflect.reference.CtExecutableReference;
Expand All @@ -23,7 +24,7 @@ public boolean isToBeProcessed(CtConstructorCall cons) {

if (cons.getType().equals(bigDecimalTypeRef)) {
List<CtExpression> expr = cons.getArguments();
if (expr.size() == 1 &&
if ((expr.size() == 1 || expr.size() == 2) &&
(expr.get(0).getType().equals(doubleTypeRef) || expr.get(0).getType().equals(floatTypeRef))) {
return true;
}
Expand All @@ -33,12 +34,21 @@ public boolean isToBeProcessed(CtConstructorCall cons) {

@Override
public void process(CtConstructorCall cons) {
CtType bigDecimalClass = getFactory().Class().get(BigDecimal.class);
CtCodeSnippetExpression invoker = getFactory().Code().createCodeSnippetExpression("BigDecimal");
CtMethod valueOfMethod = (CtMethod) bigDecimalClass.getMethodsByName("valueOf").get(0);
CtExecutableReference refToMethod = getFactory().Executable().createReference(valueOfMethod);
CtExpression arg = (CtExpression) cons.getArguments().get(0);
CtInvocation newInvocation = getFactory().Code().createInvocation(invoker, refToMethod, arg);
cons.replace(newInvocation);
if (cons.getArguments().size() == 1) {
CtType bigDecimalClass = getFactory().Class().get(BigDecimal.class);
CtCodeSnippetExpression invoker = getFactory().Code().createCodeSnippetExpression("BigDecimal");
CtMethod valueOfMethod = (CtMethod) bigDecimalClass.getMethodsByName("valueOf").get(0);
CtExecutableReference refToMethod = getFactory().Executable().createReference(valueOfMethod);
CtExpression arg = (CtExpression) cons.getArguments().get(0);
CtInvocation newInvocation = getFactory().Code().createInvocation(invoker, refToMethod, arg);
cons.replace(newInvocation);
} else {
CtConstructorCall newCtConstructorCall = cons.clone();
CtExpression arg = (CtExpression) cons.getArguments().get(0);
String argValue = arg.toString().replaceAll("[fFdD]", "");
CtLiteral<String> literal = getFactory().Code().createLiteral(argValue);
newCtConstructorCall.getArguments().set(0, literal);
cons.replace(newCtConstructorCall);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public void test() throws Exception {
String pathToRepairedFile = "./spooned/" + fileName;

JavaCheckVerifier.verify(pathToBuggyFile, new BigDecimalDoubleConstructorCheck());
Main.repair(pathToBuggyFile, Constants.PROJECT_KEY, 2111, PrettyPrintingStrategy.SNIPER);
Main.repair(pathToBuggyFile, Constants.PROJECT_KEY, 2111, PrettyPrintingStrategy.NORMAL);
TestHelper.removeComplianceComments(pathToRepairedFile);
JavaCheckVerifier.verifyNoIssue(pathToRepairedFile, new BigDecimalDoubleConstructorCheck());
}
Expand Down
22 changes: 15 additions & 7 deletions src/test/resources/ArrayHashCodeAndToString.java
Original file line number Diff line number Diff line change
@@ -1,17 +1,25 @@
/**
* Test for sonarqube rule s2116
* Arrays should not have their .toString method called as this will return the reference which is almost always wrong.
* Instead, Arrays.toString(ARRAY_VARIABLE) should be used.
*/
// Test for rule s2116

public class ArrayHashCodeAndToString {

// Tests from https://rules.sonarsource.com/java/type/Bug/RSPEC-2116
public static void main( String[] args ) {
String argStr = args.toString(); // Noncompliant
int argHash = args.hashCode(); // Noncompliant
}

// Tests from https://github.com/SonarSource/sonar-java/blob/master/java-checks-test-sources/src/main/java/checks/ArrayHashCodeAndToStringCheck.java
void method(String[] args, String string) {
Class class1 = args.getClass();
String str = string.toString();
int hash = string.hashCode();
}

// Aditional tests
public void foo(String[] args) {
String[] array1 = {"F", "O", "O"};
System.out.println(array1.toString()); // Noncompliant
varargsTest(1, 2, 3);
String argStr = args.toString(); // Noncompliant
int argHash = args.hashCode(); // Noncompliant
}

private void varargsTest(int... array2) {
Expand Down
38 changes: 22 additions & 16 deletions src/test/resources/BigDecimalDoubleConstructor.java
Original file line number Diff line number Diff line change
@@ -1,27 +1,33 @@
/**
* Test for sonarqube rule s2111.
* From https://sonarqube.ow2.org/coding_rules#rule_key=squid%3AS2111:
*
* "The results of this constructor can be somewhat unpredictable. One might assume that writing new BigDecimal(0.1)
* in Java creates a BigDecimal which is exactly equal to 0.1 (an unscaled value of 1, with a scale of 1),
* but it is actually equal to 0.1000000000000000055511151231257827021181583404541015625.
* This is because 0.1 cannot be represented exactly as a double (or, for that matter, as a binary fraction
* of any finite length). Thus, the value that is being passed in to the constructor is not exactly equal to 0.1,
* appearances notwithstanding".
*/
// Test for rule s2111

import java.math.BigDecimal;
import java.math.MathContext;

public class BigDecimalDoubleConstructor {

/*
Code taken from Sonarqube documentation https://sonarqube.ow2.org/coding_rules#rule_key=squid%3AS2111
*/
// Tests from https://rules.sonarsource.com/java/type/Bug/RSPEC-2111
public void main(String[] args) {
double d = 1.1;
BigDecimal bd1 = new BigDecimal(d); // Noncompliant; see comment above
BigDecimal bd2 = new BigDecimal(1.1); // Noncompliant; same result
}

// Tests from https://github.com/SonarSource/sonar-java/blob/master/java-checks-test-sources/src/main/java/checks/BigDecimalDoubleConstructorCheck.java
public void main2(String[] args) {
MathContext mc;
BigDecimal bd1 = new BigDecimal("1");
BigDecimal bd2 = new BigDecimal(2.0); // Noncompliant {{Use "BigDecimal.valueOf" instead.}}
BigDecimal bd4 = new BigDecimal(2.0, mc); // Noncompliant {{Use "BigDecimal.valueOf" instead.}}
BigDecimal bd5 = new BigDecimal(2.0f); // Noncompliant {{Use "BigDecimal.valueOf" instead.}}
BigDecimal bd6 = new BigDecimal(2.0f, mc); // Noncompliant {{Use "BigDecimal.valueOf" instead.}}
BigDecimal bd3 = BigDecimal.valueOf(2.0);
}

// Aditional tests
public void foo(String[] args) {
double d = 1.1;
float f = 2.2;
float f1 = 2f;
BigDecimal bd1 = new BigDecimal(d); // Noncompliant
BigDecimal bd2 = new BigDecimal(1.1); // Noncompliant
BigDecimal bd3 = new BigDecimal(f); // Noncompliant
BigDecimal bd4 = new BigDecimal(f1); // Noncompliant
BigDecimal bd5 = BigDecimal.valueOf(d); // Compliant
Expand Down
17 changes: 11 additions & 6 deletions src/test/resources/CompareStringsBoxedTypesWithEquals.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
/**
* Test for sonarqube rule s4973
* Boxed types should be compared with equals() rather than "==" since equals compares values while
* "==" compares memory location.
*/
// Test for rule s4973

public class CompareStringsBoxedTypesWithEquals {

// Test from https://rules.sonarsource.com/java/type/Bug/RSPEC-4973
public void main(String[] args) {
String firstName = getFirstName(); // String overrides equals
String lastName = getLastName();

if (firstName == lastName) { } ; // Noncompliant; false even if the strings have the same value
}

// Aditional tests
boolean eq = true;

// Java implicitly converts one variable to primitive if something boxed and primitive is compared.
Expand Down Expand Up @@ -39,7 +44,7 @@ private void nullCompare() {
}

// ENUM comparisons are excluded from transformation
private void nullCompare() {
private void nullCompare2() {
enum foo {
BAR,
XOR
Expand Down
7 changes: 3 additions & 4 deletions src/test/resources/IteratorNextException.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
/**
* Test for sonarqube rule s2272
* By contract, any implementation of Iterator.next() should throw a NoSuchElementException when there are no more elements.
*/
// Test for rule s2272

import java.util.Iterator;
import java.util.Stack;

// Test based on https://rules.sonarsource.com/java/type/Bug/RSPEC-2272
public class IteratorNextException implements Iterator {

private Stack<String> stack = new Stack();
Expand Down

0 comments on commit 44fca83

Please sign in to comment.