Feature Request: ArchRule for Comparable HashMap keys

[MahatmaFatalError]

Reference: https://stackoverflow.com/questions/70507647/how-to-assert-hashmap-keys-to-be-comparable-with-archunit/70509141

VU#903934 (Hash table implementations vulnerable to algorithmic complexity attacks) Fix with JEP180 in Java8: if too many hash codes map to the same bucket in the map, the list of entries can be changed into a balanced binary tree, sorted first by hash code and then by each key’s compareTo method, as long as the keys are Comparable.

IMHO it makes sense to provide such a rule in the standard ruleset so that others can benefit from it easily.

    @ArchTag("DoS-Protection")
    @ArchTest
    static final ArchRule fields_of_type_HashMap_should_have_Comparable_key = fields().that()
            .haveRawType(Map.class)
            .should(haveComparableFirstTypeParameter());

    @ArchTag("DoS-Protection")
    @ArchTest
    static final ArchRule code_units_should_have_parameters_of_type_Map_with_Comparable_key = codeUnits()
            .should(new ArchCondition<JavaCodeUnit>("have parameters of type Map with `Comparable` key") {
                @Override
                public void check(final JavaCodeUnit javaCodeUnit, final ConditionEvents events) {
                    javaCodeUnit.getParameters().forEach(parameter -> {
                        if (parameter.getRawType().isEquivalentTo(Map.class)) {
                            haveComparableFirstTypeParameter().check(parameter, events);
                        }
                    });
                }
            });

    @ArchTag("DoS-Protection")
    @ArchTest
    static final ArchRule methods_with_return_type_Map_should_have_return_types_with_Comparable_key = methods().that()
            .haveRawReturnType(Map.class)
            .should(new ArchCondition<JavaMethod>("have return type with `Comparable` key") {
                @Override
                public void check(final JavaMethod method, final ConditionEvents events) {
                    class ReturnType implements HasType, HasDescription {
                        @Override
                        public JavaType getType() {
                            return method.getReturnType();
                        }

                        @Override
                        public JavaClass getRawType() {
                            return method.getRawReturnType();
                        }

                        @Override
                        public String getDescription() {
                            return "Return type <" + this.getType().getName() + "> of " + method.getDescription();
                        }
                    }
                    haveComparableFirstTypeParameter().check(new ReturnType(), events);
                }
            });

    private static <T extends HasType & HasDescription> ArchCondition<T> haveComparableFirstTypeParameter() {
        return new ArchCondition<>("have `Comparable` first type parameter") {
            @Override
            public void check(final T typed, final ConditionEvents events) {
                final JavaType fieldType = typed.getType();
                if (fieldType instanceof JavaParameterizedType) {
                    final JavaType keyType = ((JavaParameterizedType) fieldType).getActualTypeArguments().get(0);

                    if (keyType instanceof JavaTypeVariable) {
                        final boolean isGenericType = true;
                        final String message = String.format(
                                "%s has a generic first type parameter %s. Comparable check skipped.",
                                typed.getDescription(), keyType.getName());
                        events.add(new SimpleConditionEvent(typed, isGenericType, message));
                    } else {
                        final JavaClass erasedType = keyType.toErasure();

                        final boolean isComparable = erasedType.getAllRawInterfaces()
                                .stream()
                                .anyMatch(rawInterface -> rawInterface.isEquivalentTo(Comparable.class));
                        final String message = String.format("%s has a first type parameter %s that %s Comparable",
                                typed.getDescription(), keyType.getName(), isComparable ? "is" : "is not");
                        events.add(new SimpleConditionEvent(typed, isComparable, message));
                    }

                } else {
                    events.add(SimpleConditionEvent.violated(typed, typed.getDescription() + " is not parameterized"));
                }
            }
        };
    }

However, for a complete check, this https://github.com/TNG/ArchUnit/issues/768 feature needs to be implemented first.

[codecholeric]

Thanks a lot for sharing this :smiley: Yes, we could definitely add this to the predefined rules, I just wonder if we should already do this without #768? Or do you think it doesn't provide enough value then? I kind of think most such relevant errors would be caught by the rule though, because the only maps left are then in a narrow local scope which probably reduces the risk of a targeted attack a lot, don't you think? If we put it to the library I would make it one rule though and explain the issue behind the rule.

[MahatmaFatalError]

I kind of think most such relevant errors would be caught by the rule though, because the only maps left are then in a narrow local scope which probably reduces the risk of a targeted attack a lot, don't you think?

Yes, agree.

If we put it to the library I would make it one rule though and explain the issue behind the rule.

Yes, makes sense.

Thanks for taking care 👍