Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to check for parameters having a certain value #721

Closed
DManstrator opened this issue Nov 19, 2021 · 2 comments
Closed

How to check for parameters having a certain value #721

DManstrator opened this issue Nov 19, 2021 · 2 comments

Comments

@DManstrator
Copy link

Hello,

currently I face that issue that I want to forbid the usage of Stream#flatMap with the parameter being Stream::distinct since it lead to a few bugs. Is this currently supported or possible somehow?

I am aware that you cannot check if the given function is equal to another function since the equals method of Object is used which checks for the identity.

Is it perhaps possible to get access to the line of code so I could work on a String to check against?

Here my current code in case it helps somehow:

    @ArchTest
    public static final ArchRule checkFlatMapStreamDistinct = ArchRuleDefinition.noClasses()
            .should(useFlatMapStreamDistinct());

    private static ArchCondition<JavaClass> useFlatMapStreamDistinct() {
        return callMethodWhere(
                DescribedPredicate.describe("use .flatMap(Stream::distinct)", methodCall -> {
                    final boolean classIsStream = methodCall.getTargetOwner().getSimpleName().equals("Stream");
                    final boolean methodIsFlatMap = methodCall.getName().equals("flatMap");
                    final boolean parameterIsStreamDistinct = false;  // TODO how to check?

                    return classIsStream && methodIsFlatMap && parameterIsStreamDistinct;
                })
        );
    }
@hankem
Copy link
Member

hankem commented Nov 19, 2021

There's two bad news for you: ArchUnit does

  1. not support method call parameter values (e.g. How to get the parameter of a method call #596).
  2. currently not support method referneces yet (Method reference is not considered as a dependency #215).

To at least offer some good news, you could probably use this to catch usages of .flatMap(stream -> stream.distinct()) :

static ArchCondition<JavaClass> useFlatMapStreamDistinct() {
    return new ArchCondition<JavaClass>("call .flatMap and Stream.distinct() in same line") {
        @Override
        public void check(JavaClass javaClass, ConditionEvents events) {
            List<JavaMethodCall> streamDistinctCalls = getMethodCallsTo(javaClass, Stream.class, "distinct");
            for (JavaMethodCall flatMapCall : getMethodCallsTo(javaClass, Stream.class, "flatMap")) {
                for (JavaMethodCall streamDistinctCall : streamDistinctCalls) {
                    SourceCodeLocation sourceCodeLocation = flatMapCall.getSourceCodeLocation();
                    boolean sameLine = sourceCodeLocation.equals(streamDistinctCall.getSourceCodeLocation());
                    if (sameLine) {  // ← Note that this check could be more liberal!
                        events.add(SimpleConditionEvent.satisfied(flatMapCall, String.format(
                                "%s calls .flatMap and Stream.distinct() in the same line in %s",
                                flatMapCall.getOrigin().getDescription(), sourceCodeLocation)
                        ));
                    }
                }
            }
        }

        List<JavaMethodCall> getMethodCallsTo(JavaClass javaClass, Class<?> methodOwner, String methodName) {
            return javaClass.getMethodCallsFromSelf().stream()
                    .filter(call -> call.getTargetOwner().isEquivalentTo(methodOwner)
                            && call.getTarget().getName().equals(methodName))
                    .collect(toList());
        }
    };
}

It can unfortunately have false positives as well as false negatives (😞), but might be effectively okay if your code style requires / prevents linebreaks accordingly. And once ArchUnit understands method references, it might actually be good enough.

@DManstrator
Copy link
Author

Hello Manfred,

thank you for your quick and informativ reply and also for your code.

However, I don't think that solution is really useable to us. There are multiple people involved in the project, telling them they have to use the non-reference style wouldn't really work out. For now we will look out for the problem in our reviews.

Still thank you very much for your help, I appreciate it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants