From 500f7e2830cf3ae78c25636b2d0169d995c693f9 Mon Sep 17 00:00:00 2001 From: timo-a Date: Mon, 22 Mar 2021 02:00:34 +0100 Subject: [PATCH] restrict fuzzy, relax getter --- README.md | 17 +- .../de/lomboker/lib/FuzzyGetterMarker.java | 2 +- .../de/lomboker/lib/FuzzySetterMarker.java | 2 +- .../main/java/de/lomboker/lib/Trivial.java | 14 ++ .../java/de/lomboker/lib/TrivialGetters.java | 19 ++- .../java/de/lomboker/lib/TrivialSetters.java | 5 +- lib/src/test/java/de/lomboker/lib/Tests.java | 85 ---------- .../java/de/lomboker/lib/TrivialTests.java | 147 ++++++++++++++++++ lib/src/test/java/de/lomboker/lib/Utils.java | 4 + .../resources/ClassBWithGetterComment.java | 2 +- .../resources/ClassBWithSetterComment.java | 2 +- .../resources/ClassWithGetterOverride.java | 10 ++ .../resources/ClassWithGetterWithThis.java | 9 ++ .../resources/ClassWithSetterWithThis.java | 17 ++ 14 files changed, 235 insertions(+), 100 deletions(-) create mode 100644 lib/src/main/java/de/lomboker/lib/Trivial.java create mode 100644 lib/src/test/java/de/lomboker/lib/TrivialTests.java create mode 100644 lib/src/test/java/de/lomboker/lib/Utils.java create mode 100644 lib/src/test/resources/ClassWithGetterOverride.java create mode 100644 lib/src/test/resources/ClassWithGetterWithThis.java create mode 100644 lib/src/test/resources/ClassWithSetterWithThis.java diff --git a/README.md b/README.md index 90a415b..9f63dd9 100644 --- a/README.md +++ b/README.md @@ -23,11 +23,22 @@ alias lomboker='java -jar ./app/build/libs/lomboker.jar' #analysis find . -name '*.java' | lomboker count > counts.txt cat counts.txt | awk '{gt+=$2; gf+=$3; st+=$4; sf+=$5} END {printf " trivial fuzzy\ngetter %5d %5d\nsetter %5d %5d\n", gt,gf,st, sf}' -cat counts.txt | awk '{print $1 $2}' | grep " 0$" | awk '{print $1}' > getterClasses.txt # reduce getters +cat counts.txt | awk '{print $1, $2}' | grep " 0$" | awk '{print $1}' > getterClasses.txt while read f; do lomboker reduce getter "$f"; done < getterClasses.txt; # put annotations on their own lines // capture first annotation (1) and white space before it (2) -cat counts.txt | xargs sed 's/^\(\(\s\{1,\}\)@\w\{1,\}\) @Getter/\1\n\2@Getter/' /tmp/test.java +cat counts.txt | xargs sed 's/^\(\(\s\{1,\}\)@\w\{1,\}(\([A-Za-z", \])\)\) @Getter/\1\n\2@Getter/' +# reduce setters +cat counts.txt | awk '{print $1, $4}' | grep " 0$" | awk '{print $1}' > setterClasses.txt +while read f; do lomboker reduce setter "$f"; done < setterClasses.txt; +# put annotations on their own lines // capture first annotation (1) and white space before it (2) +cat counts.txt | xargs sed 's/^\(\(\s\{1,\}\)@\w\{1,\}(\([A-Za-z", \])\)\) @Setter/\1\n\2@Setter/' +# mark fuzzy getters +cat counts.txt | awk '{print $1, $3}' | grep " 0$" | awk '{print $1}' > fuzzyGetters.txt +while read f; do lomboker mark getter "$f"; done < fuzzyGetters.txt; +# mark fuzzy setters +cat counts.txt | awk '{print $1, $5}' | grep " 0$" | awk '{print $1}' > fuzzySetters.txt +while read f; do lomboker mark setter "$f"; done < fuzzySetters.txt; ``` @@ -35,6 +46,4 @@ cat counts.txt | xargs sed 's/^\(\(\s\{1,\}\)@\w\{1,\}\) @Getter/\1\n\2@Getter/' - github actions - test required for push - build jars - - delete javadoc - - stop when there is annotation - getter setter shall have their own line \ No newline at end of file diff --git a/lib/src/main/java/de/lomboker/lib/FuzzyGetterMarker.java b/lib/src/main/java/de/lomboker/lib/FuzzyGetterMarker.java index d556291..e4cdbf1 100644 --- a/lib/src/main/java/de/lomboker/lib/FuzzyGetterMarker.java +++ b/lib/src/main/java/de/lomboker/lib/FuzzyGetterMarker.java @@ -13,7 +13,7 @@ public class FuzzyGetterMarker { - private static final String CHECK_COMMENT = "Lomboker says check this potential getter"; + private static final String CHECK_COMMENT = "TODO Lomboker says check this potential getter"; /** * Marks all Getters that are not trivial i.e. might need manual refactoring(renaming) first. diff --git a/lib/src/main/java/de/lomboker/lib/FuzzySetterMarker.java b/lib/src/main/java/de/lomboker/lib/FuzzySetterMarker.java index 94cdd43..18aecb2 100644 --- a/lib/src/main/java/de/lomboker/lib/FuzzySetterMarker.java +++ b/lib/src/main/java/de/lomboker/lib/FuzzySetterMarker.java @@ -13,7 +13,7 @@ public class FuzzySetterMarker { - private static final String CHECK_COMMENT = "Lomboker says check this potential setter"; + private static final String CHECK_COMMENT = "TODO Lomboker says check this potential setter"; /** * Marks all Getters that are not trivial i.e. might need manual refactoring(renaming) first. * */ diff --git a/lib/src/main/java/de/lomboker/lib/Trivial.java b/lib/src/main/java/de/lomboker/lib/Trivial.java new file mode 100644 index 0000000..3445251 --- /dev/null +++ b/lib/src/main/java/de/lomboker/lib/Trivial.java @@ -0,0 +1,14 @@ +package de.lomboker.lib; + +import com.github.javaparser.ast.body.MethodDeclaration; +import com.github.javaparser.ast.expr.MarkerAnnotationExpr; + +public class Trivial { + + public static boolean onlyTrivialAnnotations(MethodDeclaration md){ + var annotations = md.getAnnotations(); + annotations.remove(new MarkerAnnotationExpr("Override")); + return annotations.isEmpty(); + } + +} diff --git a/lib/src/main/java/de/lomboker/lib/TrivialGetters.java b/lib/src/main/java/de/lomboker/lib/TrivialGetters.java index 4e444d2..f74b684 100644 --- a/lib/src/main/java/de/lomboker/lib/TrivialGetters.java +++ b/lib/src/main/java/de/lomboker/lib/TrivialGetters.java @@ -9,7 +9,9 @@ import com.github.javaparser.ast.body.FieldDeclaration; import com.github.javaparser.ast.body.MethodDeclaration; import com.github.javaparser.ast.comments.JavadocComment; +import com.github.javaparser.ast.expr.AnnotationExpr; import com.github.javaparser.ast.expr.Expression; +import com.github.javaparser.ast.expr.MarkerAnnotationExpr; import com.github.javaparser.ast.stmt.Statement; import com.github.javaparser.javadoc.Javadoc; import com.github.javaparser.printer.lexicalpreservation.LexicalPreservingPrinter; @@ -19,7 +21,7 @@ import java.util.stream.Collectors; -public class TrivialGetters { +public class TrivialGetters extends Trivial { public static String reduceGetters(String code) { @@ -97,8 +99,13 @@ private static Optional getReturnStatement(MethodDeclaration md) { Optional oExpression = onlyStmt.asReturnStmt().getExpression(); - if (oExpression.isEmpty() - || oExpression.get().toString().contains(" ")) + if (oExpression.isEmpty()) + return Optional.empty(); + + var rExpr = oExpression.get().toString(); + if ( rExpr.contains(" ") + || !rExpr.startsWith("this.") && rExpr.contains(".") // return member.getX() + ) return Optional.empty(); return oExpression; @@ -109,7 +116,7 @@ public static boolean isTrivialGetter(MethodDeclaration md, Set fields) return false; } - if (!md.getAnnotations().isEmpty()) + if (!onlyTrivialAnnotations(md)) return false; //Verify that the name is what lombok would create @@ -121,6 +128,7 @@ public static boolean isTrivialGetter(MethodDeclaration md, Set fields) return true; } + //assumptions: type is not void private static boolean nameMatch(String methodName, String type, String variable) { @@ -146,7 +154,8 @@ private static String getReturned(MethodDeclaration md) { return md.getBody().get().getStatements().stream() .filter(Statement::isReturnStmt) .reduce((first, second) -> second).get() //last element - .asReturnStmt().getExpression().get().toString(); + .asReturnStmt().getExpression().get().toString() + .replace("this.", ""); } /* Count number of getters */ diff --git a/lib/src/main/java/de/lomboker/lib/TrivialSetters.java b/lib/src/main/java/de/lomboker/lib/TrivialSetters.java index 24001a1..8fa7512 100644 --- a/lib/src/main/java/de/lomboker/lib/TrivialSetters.java +++ b/lib/src/main/java/de/lomboker/lib/TrivialSetters.java @@ -11,6 +11,7 @@ import com.github.javaparser.ast.body.Parameter; import com.github.javaparser.ast.expr.AssignExpr; import com.github.javaparser.ast.expr.Expression; +import com.github.javaparser.ast.expr.MarkerAnnotationExpr; import com.github.javaparser.ast.stmt.ExpressionStmt; import com.github.javaparser.ast.stmt.Statement; import com.github.javaparser.printer.lexicalpreservation.LexicalPreservingPrinter; @@ -20,7 +21,7 @@ import java.util.stream.Collectors; -public class TrivialSetters { +public class TrivialSetters extends Trivial { public static String reduceSetters(String code) { @@ -150,7 +151,7 @@ public static boolean isTrivialSetter(MethodDeclaration md, Set fields) return false; } - if (!md.getAnnotations().isEmpty()) + if (!onlyTrivialAnnotations(md)) return false; //Verify that the name is what lombok would create diff --git a/lib/src/test/java/de/lomboker/lib/Tests.java b/lib/src/test/java/de/lomboker/lib/Tests.java index ef34aee..30bf098 100644 --- a/lib/src/test/java/de/lomboker/lib/Tests.java +++ b/lib/src/test/java/de/lomboker/lib/Tests.java @@ -12,91 +12,6 @@ public class Tests { - @Test - public void testGetter() throws IOException { - String fileName = "ClassAWithGetterInput.java"; - String fileNameRef = "ClassAWithLombokGetter.java"; - String input = readFile(fileName); - String expected = readFile(fileNameRef); - - assertEquals(expected, TrivialGetters.reduceGetters(input)); - - } - - @Test - public void testSetter() throws IOException { - String fileName = "ClassAWithSetterInput.java"; - String fileNameRef = "ClassAWithLombokSetter.java"; - String input = readFile(fileName); - String expected = readFile(fileNameRef); - - assertEquals(expected, TrivialSetters.reduceSetters(input)); - - } - - @Test - public void shouldRemoveJavadocForGetter() throws IOException { - String fileName = "ClassWithGetterWithJavadoc.java"; - String fileNameRef = "ClassWithGetterWithJavadoc.reduced.g.java"; - String input = readFile(fileName); - String expected = readFile(fileNameRef); - - assertEquals(expected, TrivialGetters.reduceGetters(input)); - } - - @Test - public void shouldRemoveJavadocForSetter() throws IOException { - String fileName = "ClassWithSetterWithJavadoc.java"; - String fileNameRef = "ClassWithSetterWithJavadoc.reduced.s.java"; - String input = readFile(fileName); - String expected = readFile(fileNameRef); - - assertEquals(expected, TrivialSetters.reduceSetters(input)); - } - - @Test - public void shouldRejectGetterWithAnnotation() throws IOException { - String fileName = "ClassWithGetterWithAnnotation.java"; - String input = readFile(fileName); - ClassWrapper cw = new ClassWrapper(input); - - assertFalse(TrivialGetters.isTrivialGetter(cw.methods.get(0), cw.fieldNames)); - } - - @Test - public void shouldRejectSetterWithAnnotation() throws IOException { - String fileName = "ClassWithSetterWithAnnotation.java"; - String input = readFile(fileName); - ClassWrapper cw = new ClassWrapper(input); - - assertFalse(TrivialSetters.isTrivialSetter(cw.methods.get(0), cw.fieldNames)); - - } - - //@Test functionality not implemented https://stackoverflow.com/questions/66698723/how-to-put-annotations-on-a-new-line-with-javaparser - // workaround: use command line tools - public void getterReducerShouldPlaceAnnotationOnNewLine() throws IOException { - String fileName = "ClassWithGetterWithAnnotationOnField.java"; - String fileNameRef = "ClassWithGetterWithAnnotationOnField.reduced.g.java"; - String input = readFile(fileName); - String expected = readFile(fileNameRef); - - assertEquals(expected, TrivialGetters.reduceGetters(input)); - - } - - //@Test - public void setterReducerShouldPlaceAnnotationOnNewLine() throws IOException { - String fileName = "ClassWithSetterWithAnnotationOnField.java"; - String fileNameRef = "ClassWithSetterWithAnnotationOnField.reduced.s.java"; - String input = readFile(fileName); - String expected = readFile(fileNameRef); - - assertEquals(expected, TrivialSetters.reduceSetters(input)); - - } - - @Test public void testGetterMarker() throws IOException { String fileName = "ClassBWithGetterInput.java"; diff --git a/lib/src/test/java/de/lomboker/lib/TrivialTests.java b/lib/src/test/java/de/lomboker/lib/TrivialTests.java new file mode 100644 index 0000000..1920326 --- /dev/null +++ b/lib/src/test/java/de/lomboker/lib/TrivialTests.java @@ -0,0 +1,147 @@ +package de.lomboker.lib; + +import org.junit.jupiter.api.Test; + +import java.io.File; +import java.io.IOException; +import java.net.URL; +import java.nio.file.Files; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +public class TrivialTests { + + @Test + public void testGetter() throws IOException { + String fileName = "ClassAWithGetterInput.java"; + String fileNameRef = "ClassAWithLombokGetter.java"; + String input = readFile(fileName); + String expected = readFile(fileNameRef); + + assertEquals(expected, TrivialGetters.reduceGetters(input)); + + } + + @Test + public void testSetter() throws IOException { + String fileName = "ClassAWithSetterInput.java"; + String fileNameRef = "ClassAWithLombokSetter.java"; + String input = readFile(fileName); + String expected = readFile(fileNameRef); + + assertEquals(expected, TrivialSetters.reduceSetters(input)); + + } + + @Test + public void shouldRecognizeGetterWithThis() throws IOException { + String fileName = "ClassWithGetterWithThis.java"; + String fileNameRef = "ClassAWithLombokGetter.java"; + String input = readFile(fileName); + String expected = readFile(fileNameRef); + + assertEquals(expected, TrivialGetters.reduceGetters(input)); + + } + + @Test + public void shouldRecognizeSetterWithThis() throws IOException { + String fileName = "ClassAWithSetterInput.java"; + String fileNameRef = "ClassAWithLombokSetter.java"; + String input = readFile(fileName); + String expected = readFile(fileNameRef); + + assertEquals(expected, TrivialSetters.reduceSetters(input)); + + } + + @Test + public void shouldRemoveJavadocForGetter() throws IOException { + String fileName = "ClassWithGetterWithJavadoc.java"; + String fileNameRef = "ClassWithGetterWithJavadoc.reduced.g.java"; + String input = readFile(fileName); + String expected = readFile(fileNameRef); + + assertEquals(expected, TrivialGetters.reduceGetters(input)); + } + + @Test + public void shouldRemoveJavadocForSetter() throws IOException { + String fileName = "ClassWithSetterWithJavadoc.java"; + String fileNameRef = "ClassWithSetterWithJavadoc.reduced.s.java"; + String input = readFile(fileName); + String expected = readFile(fileNameRef); + + assertEquals(expected, TrivialSetters.reduceSetters(input)); + } + + @Test + public void shouldRejectGetterWithAnnotation() throws IOException { + String fileName = "ClassWithGetterWithAnnotation.java"; + String input = readFile(fileName); + ClassWrapper cw = new ClassWrapper(input); + + assertFalse(TrivialGetters.isTrivialGetter(cw.methods.get(0), cw.fieldNames)); + } + + @Test + public void shouldRejectSetterWithAnnotation() throws IOException { + String fileName = "ClassWithSetterWithAnnotation.java"; + String input = readFile(fileName); + ClassWrapper cw = new ClassWrapper(input); + + assertFalse(TrivialSetters.isTrivialSetter(cw.methods.get(0), cw.fieldNames)); + + } + + @Test + public void shouldAllowOverrideAsAnnotation() throws IOException { + String fileName = "ClassWithGetterOverride.java"; + String fileNameRef = "ClassAWithLombokGetter.java"; + String input = readFile(fileName); + String expected = readFile(fileNameRef); + + assertEquals(expected, TrivialGetters.reduceGetters(input)); + + } + + + //@Test functionality not implemented https://stackoverflow.com/questions/66698723/how-to-put-annotations-on-a-new-line-with-javaparser + // workaround: use command line tools + public void getterReducerShouldPlaceAnnotationOnNewLine() throws IOException { + String fileName = "ClassWithGetterWithAnnotationOnField.java"; + String fileNameRef = "ClassWithGetterWithAnnotationOnField.reduced.g.java"; + String input = readFile(fileName); + String expected = readFile(fileNameRef); + + assertEquals(expected, TrivialGetters.reduceGetters(input)); + + } + + //@Test + public void setterReducerShouldPlaceAnnotationOnNewLine() throws IOException { + String fileName = "ClassWithSetterWithAnnotationOnField.java"; + String fileNameRef = "ClassWithSetterWithAnnotationOnField.reduced.s.java"; + String input = readFile(fileName); + String expected = readFile(fileNameRef); + + assertEquals(expected, TrivialSetters.reduceSetters(input)); + + } + + + + private String readFile(String fileName) throws IOException { + ClassLoader classLoader = getClass().getClassLoader(); + URL resource = classLoader.getResource(fileName); + + if (resource == null) + throw new IllegalArgumentException("file not found! " + fileName); + + File f = new File(resource.getFile()); + + return Files.readString(f.toPath()); + } + +} diff --git a/lib/src/test/java/de/lomboker/lib/Utils.java b/lib/src/test/java/de/lomboker/lib/Utils.java new file mode 100644 index 0000000..d28cb3b --- /dev/null +++ b/lib/src/test/java/de/lomboker/lib/Utils.java @@ -0,0 +1,4 @@ +package de.lomboker.lib; + +public class Utils { +} diff --git a/lib/src/test/resources/ClassBWithGetterComment.java b/lib/src/test/resources/ClassBWithGetterComment.java index b951750..0f7b299 100644 --- a/lib/src/test/resources/ClassBWithGetterComment.java +++ b/lib/src/test/resources/ClassBWithGetterComment.java @@ -2,7 +2,7 @@ class A { private int number = 5; - //Lomboker says check this potential getter + //TODO Lomboker says check this potential getter public int getTheNumber() { return number; } diff --git a/lib/src/test/resources/ClassBWithSetterComment.java b/lib/src/test/resources/ClassBWithSetterComment.java index fd39c6d..0ae8dff 100644 --- a/lib/src/test/resources/ClassBWithSetterComment.java +++ b/lib/src/test/resources/ClassBWithSetterComment.java @@ -2,7 +2,7 @@ class A { private int number; - //Lomboker says check this potential setter + //TODO Lomboker says check this potential setter public void setTheNumber(int i) { number = i; } diff --git a/lib/src/test/resources/ClassWithGetterOverride.java b/lib/src/test/resources/ClassWithGetterOverride.java new file mode 100644 index 0000000..2058dae --- /dev/null +++ b/lib/src/test/resources/ClassWithGetterOverride.java @@ -0,0 +1,10 @@ +class A { + + private int number = 5; + + @Override + public int getNumber() { + return number; + } + +} diff --git a/lib/src/test/resources/ClassWithGetterWithThis.java b/lib/src/test/resources/ClassWithGetterWithThis.java new file mode 100644 index 0000000..8cff850 --- /dev/null +++ b/lib/src/test/resources/ClassWithGetterWithThis.java @@ -0,0 +1,9 @@ +class A { + + private int number = 5; + + public int getNumber() { + return this.number; + } + +} diff --git a/lib/src/test/resources/ClassWithSetterWithThis.java b/lib/src/test/resources/ClassWithSetterWithThis.java new file mode 100644 index 0000000..6387189 --- /dev/null +++ b/lib/src/test/resources/ClassWithSetterWithThis.java @@ -0,0 +1,17 @@ +import de.lomboker.lib.TrivialGetters; +import de.lomboker.lib.TrivialSetters; +import org.junit.jupiter.api.Test; + +import java.io.IOException; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class A { + + private int number; + + public void setNumber(int i) { + this.number = i; + } + +}