TNG / ArchUnit

A Java architecture test library, to specify and assert architecture rules in plain Java
http://archunit.org
Apache License 2.0
3.21k stars 293 forks source link

How to test for getters and setters #349

Open Agnieszka321 opened 4 years ago

Agnieszka321 commented 4 years ago

Hi I need to write test checking if all fields in the class have getter and setters for them + only default constructors. Is it possible with predefined checks or do I need to implement rule with custom concept?

codecholeric commented 4 years ago

You mean like a Java Bean standard test, right? Unfortunately that doesn't exist yet :frowning_face: (I'm also not a big fan of the standard, i.g., but I know that in some contexts you have no choice). So I guess you would have to extend the API yourself, like

classes().that(areJavaBeans())
    .should(adhereToJavaBeansStandard())

// ...

private ArchCondition<JavaClass> adhereToJavaBeansStandard() {
    return new ArchCondition<JavaClass>("adhere to Java Beans standard") {
        @Override
        public void check(final JavaClass javaClass, ConditionEvents events) {
            javaClass.getFields()
                    .forEach(f -> {
                        // make this smarter if considered necessary ;-)
                        if (!javaClass.tryGetMethod("get" + capitalize(f.getName())).isPresent()) {
                            events.add(SimpleConditionEvent.violated(f, "Field " + f.getFullName() + " has no Getter"));
                        }
                        if (!javaClass.tryGetMethod("set" + capitalize(f.getName())).isPresent()) {
                            events.add(SimpleConditionEvent.violated(f, "Field " + f.getFullName() + " has no Setter"));
                        }
                    });
        }
    };
}

The case of the default constructor is described here -> https://github.com/TNG/ArchUnit/issues/322

Sorry that there is no more convenient way :disappointed: Feel free to contribute a PR for com.tngtech.archunit.library (compare GeneralCodingRules) if you want to, I'll happily merge it :smiley:

hankem commented 4 years ago

Not sure whether that's relevant from a practical perspective, but I realized that the DescribedPredicate haveNoParameters given in the other issue is formally not suited to select default constructors, which for (non-static) inner classes actually do have a parameter – the enclosing class.

Trying to model some more properties of a default constructor (to reduce the number of false-positives), I've so far arrived at:

static ArchCondition<JavaClass> onlyHaveADefaultConstructor = new ArchCondition<JavaClass>("only have a default constructor") {
    @Override
    public void check(JavaClass javaClass, ConditionEvents events) {
        Set<JavaConstructor> constructors = javaClass.getConstructors();
        if (constructors.size() != 1) {
            events.add(SimpleConditionEvent.violated(javaClass,
                    String.format("%s does not have only 1 constructor", javaClass.getName())));
            return;
        }

        JavaConstructor constructor = constructors.iterator().next();
        if (!constructor.getThrowsClause().getTypes().isEmpty()) {
            events.add(SimpleConditionEvent.violated(javaClass,
                    String.format("%s has a throws clause %s",
                            constructor.getFullName(), constructor.getSourceCodeLocation())));
            return;
        }

        JavaModifier constructorAccessModifier = null, classAccessModifier = null;
        for (JavaModifier modifier : asList(JavaModifier.PUBLIC, JavaModifier.PROTECTED, JavaModifier.PRIVATE)) {
            if (constructor.getModifiers().contains(modifier)) {
                constructorAccessModifier = modifier;
            }
            if (javaClass.getModifiers().contains(modifier)) {
                classAccessModifier = modifier;
            }
        }
        if (classAccessModifier != constructorAccessModifier) {
            events.add(SimpleConditionEvent.violated(javaClass,
                    String.format("%s has another access modifier than its class %s",
                            constructor.getFullName(), constructor.getSourceCodeLocation())));
            return;
        }

        int expectedNumberOfParameters = javaClass.isInnerClass() ? 1 : 0;
        if (constructor.getRawParameterTypes().size() != expectedNumberOfParameters) {
            events.add(SimpleConditionEvent.violated(javaClass,
                    String.format("%s does not have %d parameters %s",
                            constructor.getFullName(), expectedNumberOfParameters, constructor.getSourceCodeLocation())));
            return;
        }

        events.add(SimpleConditionEvent.satisfied(javaClass,
                String.format("%s seems to have a default constructor %s",
                        javaClass.getName(), constructor.getSourceCodeLocation())));
    }
};

This would still miss classes with a single non-default constructor that has no throws clauses, the same access modifier and parameter lists as a default constructor would have, but does actually contain non-default code. Maybe those could be caught by checking constructor.getAccessesFromSelf().isEmpty()?

codecholeric commented 4 years ago

@hankem thanks for the update :+1: This looks more and more like it should actually be part of the lang API out of the box, so nobody else has to go through this :rofl: I wonder though, is the case "default constructor" really the relevant one? In all cases I can think of the only relevant characteristic is a public constructor with no arguments (and actually inner classes usually don't play a role there either, because this usually applies to some framework code, where inner classes don't really make any sense)

hankem commented 4 years ago

@codecholeric, I agree that for many use cases, one might just want to ensure that (non-nested) classes have a parameterless (and potentially also public) constructor. But in theory, one could also want to ensure that no constructors are implemented manually; regardless of whether one would call this an architecture or rather a code style rule, ArchUnit allows to formulate it. I'll try to contribute a PR once I manage to get back to it.

slaverda commented 3 years ago

Hi team,

What about default/package modifier ? At this point of time the only way to check that some method has a default/package modifier is something like:

List<JavaModifier> accessModifiers = List.of(JavaModifier.PRIVATE, JavaModifier.PROTECTED, JavaModifier.PUBLIC); boolean isPackagePrivate = !field.getAccessesToSelf().stream().filter(it -> it.getAccessType() == JavaFieldAccess.AccessType.SET).collect(Collectors.toList()).get(0).getOwner().getModifiers().containsAll(accessModifiers);

Is there any smarter solution ?

Regards, Slavi

hankem commented 3 years ago

@slaverda I guess that getModifiers().containsAll(accessModifiers) will rarely be true... 😉

Is your question whether HasModifiers should have something like this?

default boolean isPackagePrivate() {
    Set<JavaModifier> accessModifiers = EnumSet.of(JavaModifier.PRIVATE, JavaModifier.PROTECTED, JavaModifier.PUBLIC);
    return com.google.common.collect.Sets.intersection(accessModifiers, getModifiers()).isEmpty();
}

(As ArchUnit is currently compatible with Java 7, it can't support default methods, but this is just to illustrate the idea...)