From 4d297630e5b639a899e1d469c85aa7e439eae936 Mon Sep 17 00:00:00 2001 From: Lucas <24826124+Luro02@users.noreply.github.com> Date: Thu, 24 Aug 2023 12:15:46 +0200 Subject: [PATCH] implement #217 --- .../core/check/complexity/RegexCheck.java | 38 +++++++++++++++- .../core/check_tests/Regex/code/Test.java | 43 +++++++++++++++++++ .../core/check_tests/Regex/config.txt | 17 +++++--- 3 files changed, 91 insertions(+), 7 deletions(-) diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RegexCheck.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RegexCheck.java index 16834c10..ad84d0cc 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RegexCheck.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RegexCheck.java @@ -24,10 +24,14 @@ import de.firemage.autograder.treeg.ast.RegExCharacter; import de.firemage.autograder.treeg.ast.RegExNode; import spoon.processing.AbstractProcessor; +import spoon.reflect.code.CtInvocation; import spoon.reflect.code.CtLiteral; -import spoon.reflect.code.CtStatement; +import spoon.reflect.code.CtTypeAccess; +import spoon.reflect.code.CtVariableAccess; import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtVariable; +import spoon.reflect.reference.CtExecutableReference; +import spoon.reflect.visitor.filter.VariableAccessFilter; import java.util.List; import java.util.Map; @@ -49,13 +53,43 @@ private static boolean looksLikeRegex(String value) { return REGEX_HINTS.stream().filter(value::contains).count() >= MIN_REGEX_HINTS; } + private static boolean isRegexInvocation(CtInvocation ctInvocation) { + CtExecutableReference ctExecutable = ctInvocation.getExecutable(); + + return ctInvocation.getTarget() instanceof CtTypeAccess ctTypeAccess + && SpoonUtil.isTypeEqualTo(ctTypeAccess.getAccessedType(), java.util.regex.Pattern.class) + && List.of("matches", "compile").contains(ctExecutable.getSimpleName()) + || SpoonUtil.isTypeEqualTo(ctInvocation.getTarget().getType(), java.lang.String.class) + && ( + SpoonUtil.isSignatureEqualTo(ctExecutable, boolean.class, "matches", String.class) + || SpoonUtil.isSignatureEqualTo(ctExecutable, String.class, "replaceAll", String.class, String.class) + || SpoonUtil.isSignatureEqualTo(ctExecutable, String.class, "replaceFirst", String.class, String.class) + || SpoonUtil.isSignatureEqualTo(ctExecutable, String[].class, "split", String.class) + || SpoonUtil.isSignatureEqualTo(ctExecutable, String[].class, "split", String.class, int.class) + ); + } + + private static boolean isInAllowedContext(CtLiteral ctLiteral) { + CtElement parent = ctLiteral.getParent(); + if (parent instanceof CtVariable ctVariable + && SpoonUtil.isEffectivelyFinal(ctVariable.getReference())) { + List> invocations = parent.getFactory().getModel().getElements(new VariableAccessFilter<>(ctVariable.getReference())); + + return !invocations.isEmpty() && + invocations + .stream() + .allMatch(ctVariableAccess -> ctVariableAccess.getParent() instanceof CtInvocation ctInvocation && isRegexInvocation(ctInvocation)); + } + + return parent instanceof CtInvocation ctInvocation && isRegexInvocation(ctInvocation); + } @Override protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnalysis) { staticAnalysis.processWith(new AbstractProcessor>() { @Override public void process(CtLiteral literal) { - if (!SpoonUtil.isString(literal.getType())) { + if (!SpoonUtil.isString(literal.getType()) || !isInAllowedContext(literal)) { return; } diff --git a/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/code/Test.java b/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/code/Test.java index 4de146e0..01e7565f 100644 --- a/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/code/Test.java +++ b/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/code/Test.java @@ -1,3 +1,7 @@ +package de.firemage.autograder.core.check_tests.Regex.code; + +import java.util.regex.Pattern; + public class Test { private String noRegex = "Should we do this? I guess we shouldn't! f*ck you!"; private String regex1 = "(foo)* [bar]+ x? x?"; /*# not ok #*/ @@ -19,4 +23,43 @@ public class Test { private static final String FORMAT_STRING_1 = "coordinate (%s, %s) is invalid!"; private static final String FORMAT_STRING_2 = "coordinate (%s, %s)\n is invalid?\n"; + + private void foo() { + Pattern pattern = Pattern.compile(regex1); + pattern = Pattern.compile(regex2); + pattern = Pattern.compile(regex3); + pattern = Pattern.compile(regex4); + pattern = Pattern.compile(regex5); + pattern = Pattern.compile(simpleRegex1); + pattern = Pattern.compile(simpleRegex2); + pattern = Pattern.compile(simpleRegex3); + pattern = Pattern.compile(invalidRegex); + pattern = Pattern.compile(COMPLICATED_REGEX_1); + pattern = Pattern.compile(COMPLICATED_REGEX_2); + pattern = Pattern.compile(FORMAT_STRING_1); + pattern = Pattern.compile(FORMAT_STRING_2); + } +} + +// test that context of regex is considered +class RegexContext { + private static final String DEFINITELY_REGEX_1 = "(foo)* [bar]+ x? x?"; /*# not ok #*/ + private static final String DEFINITELY_REGEX_2 = "(foo)* [bar]+ x? x?"; /*# not ok #*/ + private static final String DEFINITELY_REGEX_3 = "(foo)* [bar]+ x? x?"; /*# not ok #*/ + private static final String DEFINITELY_REGEX_4 = "(foo)* [bar]+ x? x?"; /*# not ok #*/ + private static final String DEFINITELY_REGEX_5 = "(foo)* [bar]+ x? x?"; /*# not ok #*/ + private static final String DEFINITELY_REGEX_6 = "(foo)* [bar]+ x? x?"; /*# not ok #*/ + private static final String UNUSED_REGEX = "(foo)* [bar]+ x? x?"; /*# ok #*/ + private static final String NOT_USED_AS_REGEX = "(foo)* [bar]+ x? x?"; /*# ok #*/ + + void foo() { + boolean matches = Pattern.matches(DEFINITELY_REGEX_1, "foo bar x"); + matches = "foo bar x".matches(DEFINITELY_REGEX_2); + String f = "foo bar x".replaceAll(DEFINITELY_REGEX_3, "foo bar x"); + f = "foo bar x".replaceFirst(DEFINITELY_REGEX_4, "foo bar x"); + String[] parts = "foo bar x".split(DEFINITELY_REGEX_5); + parts = "foo bar x".split(DEFINITELY_REGEX_6, -1); + + System.out.println(NOT_USED_AS_REGEX); + } } diff --git a/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/config.txt b/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/config.txt index 231a26e3..9f9b1df5 100644 --- a/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/config.txt +++ b/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/config.txt @@ -1,7 +1,14 @@ complexity.RegexCheck Complex regex -Test.java:3 -Test.java:4 -Test.java:5 -Test.java:6 -Test.java:7 \ No newline at end of file +Test.java:7 +Test.java:8 +Test.java:9 +Test.java:10 +Test.java:11 + +Test.java:46 +Test.java:47 +Test.java:48 +Test.java:49 +Test.java:50 +Test.java:51