Skip to content

Commit

Permalink
remove Optional parameter check #527
Browse files Browse the repository at this point in the history
  • Loading branch information
Luro02 committed May 16, 2024
1 parent e12a850 commit 56ceb71
Show file tree
Hide file tree
Showing 7 changed files with 3 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ public enum ProblemType {
AVOID_INNER_CLASSES,
USE_STRING_FORMATTED,
OPTIONAL_TRI_STATE,
OPTIONAL_ARGUMENT,
AVOID_LABELS,
SIMPLIFY_ARRAYS_FILL,
REDUNDANT_ASSIGNMENT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@
import de.firemage.autograder.core.integrated.StaticAnalysis;
import spoon.processing.AbstractProcessor;
import spoon.reflect.declaration.CtMethod;
import spoon.reflect.declaration.CtParameter;
import spoon.reflect.declaration.CtTypedElement;
import spoon.reflect.declaration.CtVariable;
import spoon.reflect.reference.CtTypeReference;

@ExecutableCheck(reportedProblems = { ProblemType.OPTIONAL_TRI_STATE, ProblemType.OPTIONAL_ARGUMENT })
@ExecutableCheck(reportedProblems = { ProblemType.OPTIONAL_TRI_STATE })
public class OptionalBadPractices extends IntegratedCheck {
private void checkCtVariable(CtTypedElement<?> ctTypedElement) {
CtTypeReference<?> ctTypeReference = ctTypedElement.getType();
Expand All @@ -24,15 +23,6 @@ private void checkCtVariable(CtTypedElement<?> ctTypedElement) {
return;
}

// Check if the variable is a function parameter:
if (ctTypedElement instanceof CtParameter<?>) {
this.addLocalProblem(
ctTypeReference,
new LocalizedMessage("optional-argument"),
ProblemType.OPTIONAL_ARGUMENT
);
}

// Check if the Optional is used as a tri-state:
boolean isTriState =
ctTypeReference.getActualTypeArguments().stream()
Expand Down
1 change: 0 additions & 1 deletion autograder-core/src/main/resources/strings.de.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ common-reimplementation = Der Code kann vereinfacht werden zu '{$suggestion}'.
use-string-formatted = '{$formatted}' ist schöner zu lesen.
use-format-string = '{$formatted}' ist schöner zu lesen.
optional-argument = Optional sollte nicht als Argument verwendet werden, da man dann 3 Zustände hat: null, Optional.empty() und Optional.of(..). Siehe https://stackoverflow.com/a/31924845/7766117
optional-tri-state = Statt einem Optional boolean, sollte man ein enum verwenden.
equals-hashcode-comparable-contract = Es müssen immer equals und hashCode zusammen überschrieben werden. Genauso muss wenn Comparable implementiert wird equals und hashCode überschrieben werden.
Expand Down
1 change: 0 additions & 1 deletion autograder-core/src/main/resources/strings.en.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ common-reimplementation = The code can be simplified to '{$suggestion}'.
use-string-formatted = '{$formatted}' is easier to read.
use-format-string = '{$formatted}' is easier to read.
optional-argument = Optional should not be used as an argument, because it has 3 states: null, Optional.empty() and Optional.of(..). See https://stackoverflow.com/a/31924845/7766117
optional-tri-state = Instead of an Optional boolean, one should use an enum.
equals-hashcode-comparable-contract = Equals and hashCode must always be overridden together. Similarly for Comparable, both equals and hashCode must be overwritten.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
public class Test {
private Optional<String> opt1; /*# ok #*/

public Test(Optional<String> opt1) { /*# not ok #*/
this.opt1 = opt1;
}

public static void main(String[] args) {
Optional<String> optional = Optional.empty(); /*# ok #*/
Optional<Boolean> optionalTriState = Optional.of(true); /*# not ok #*/
Expand All @@ -21,8 +17,4 @@ public Optional<String> getOpt1() { /*# ok #*/
public Optional<Boolean> getOpt2() { /*# not ok #*/
return Optional.empty();
}

public Optional<String> setOpt3(Optional<Integer> a) { /*# not ok #*/
return Optional.empty();
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
api.OptionalBadPractices
Check for bad practices in Optional usage.
Test.java:8
Test.java:14
Test.java:21
Test.java:25
Test.java:10
Test.java:17
1 change: 0 additions & 1 deletion sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ problemsToReport:
- DO_NOT_USE_SYSTEM_EXIT
- AVOID_INNER_CLASSES
- USE_STRING_FORMATTED
- OPTIONAL_ARGUMENT
- OPTIONAL_TRI_STATE
- AVOID_LABELS
- AVOID_SHADOWING
Expand Down

0 comments on commit 56ceb71

Please sign in to comment.