jqassistant-archive/jqa-spring-plugin

jqa-spring-plugin's Issues

Rule for "A Spring service can only have dependencies to other Spring components that are services or repositories" is too strict

Bug Description

When you have a Service which uses a MapStruct Mapper where one enitity will be mapped to a new format you got this message, becauce MapStruct generated @Component

Expected Behaviour

Don't show this Error on Components

How can we reproduce the bug?

Create a Service which uses MapStruct to Map from one format to the other and scan this Service. Or just checkout

https://github.com/TheSasch/jqa_spring_example

to see a demo application which shows the problem within the analysis. Use

mvnw clean verify -P jqassistant

to see the problem.

Definition of Done for the Implementers

• Integration tests have been written (if applicable)
• Test coverage is the same or even better then before (if applicable)
• Documentation has been written (if applicable)
• Added a note on the new feature to the release notes (if applicable)

New Constraint: Injectables must only be held in Injectables

Feature Description

It's possible to hold Injectables as a field in classes, which are not maneged by Spring. With that, classes that are manually injected hold classes that are managed by Spring.

For that I propose to add a new (strict) constraint spring-injection:InjectablesMustOnlyBeHeldInInjectables

How to Test

Create a bean producer returning any JDK class. If the constraint fails, the ticket is done.

Definition of Done for the Implementers

• We got a final feedback from the reporting user (if applicable)
• Unittests have been written (if applicable)
• Integration tests have been written (if applicable)
• Test coverage is the same or even better then before (if applicable)
• Documentation has been written (if applicable)
• Added a note on the new feature to the release notes (if applicable)

Constraint InjectablesMustNotBeInstantiated is too strict

In my projects, I'm getting InjectablesMustNotBeInstantiated violations for a pattern we use frequently:

@Configuration
public class JacksonConfig {

    @Bean
    public ObjectMapper objectMapper() {
        return ObjectMapperFactory.createObjectMapper();
    }
}

public class ObjectMapperFactory {

    public static ObjectMapper createObjectMapper() {
        ObjectMapper objectMapper = new ObjectMapper();
        objectMapper.registerModules(new JavaTimeModule());
        return objectMapper;
    }
}

The static method allows us to use the same ObjectMapper definition both in Spring and non-Spring contexts (where non-Spring is not just test code - it is not unusual to have some other frameworks or libs within a Spring application which do not work with Spring beans).

Moving the constructor call to the producer method and then calling the producer method from createObjectMapper() would avoid the violation, but not really improve the code by introducing a dependency of the ObjectMapperFactory on Spring.

So my proposal is to rewrite the violation condition along the lines of "If the constructor of an Injectable is called from another Injectable, then the caller must be a BeanProducer".

MATCH
    (injectable:Injectable:Type)-[:DECLARES]->(constructor:Constructor),
    (type:Injectable:Type)-[:DECLARES]->(method:Method)-[newInstance:INVOKES*1..]->(constructor)
WHERE NOT (method:Spring:BeanProducer)

Consumer present in @TestConfiguration reported as BeanProducerMustBeDeclaredInConfigurationComponent

Given I have test configuration in my spock test class

@TestConfiguration
static class SpockDetachedMockConfig {
	def mockFactory = new DetachedMockFactory()

	@Bean
	Consumer<CacheUpdate> cacheUpdateNotificator() {
		return mockFactory.Mock(Consumer)
	}
}	

it is reported as

[ERROR] Constraint: spring-injection:BeanProducerMustBeDeclaredInConfigurationComponent
[ERROR] Severity: MAJOR
[ERROR] Bean producer methods must be declared in configuration components.

Constraint: spring-injection:InjectablesMustOnlyBeHeldInInjectables - false positives?

First: Thanks a lot for providing jQAssistant.

Bug Description

The constraint spring-injection:InjectablesMustOnlyBeHeldInInjectables detects non-static inner classes of injectables, such inner classes always hold a reference to its owning class.

Expected Behaviour

Please modify this constraint.

How can we reproduce the bug?

Example 1:

@Configuration
class ConfigurationWithInnerClass {

	@Bean
	HttpComponentsClientHttpRequestFactory httpRequestFactoryUsingInnerClass() {
		try {
			return new HttpComponentsClientHttpRequestFactory(HttpClientBuilder.create().useSystemProperties()
					.setSSLContext(new SSLContextBuilder().loadTrustMaterial(new TrustStrategy() {

						@Override
						public boolean isTrusted(X509Certificate[] chain, String authType) throws CertificateException {
							return true;
						}
					}).build()).build());
		} catch (KeyManagementException | NoSuchAlgorithmException | KeyStoreException e) {
			throw new RuntimeException(e);
		}
	}
}

results in NonInjectableHavingInjectablesAsField=ConfigurationWithInnerClass$1, Fields=ConfigurationWithInnerClass

Example 2:

@Component
public class ComponentWithInnerClass {

	private class InnerClassUsingSomeMethodFromOuterClass {

	}

}

results in NonInjectableHavingInjectablesAsField=ComponentWithInnerClass$InnerClassUsingSomeMethodFromOuterClass, Fields=ComponentWithInnerClass

Constraint: TransactionMethodMustNotBeCalledDirectly

[[structure:TransactionMethodMustNotBeCalledDirectly]]
.Transaktionale Methoden dürfen innerhalb einer Klasse nicht von anderen Methoden aufgerufen werden
[source,cypher,role=constraint,requiresConcepts="spring:TransactionMethod"]

Constraint: NoFieldInjectionInConfiguration

[[structure:NoFieldInjectionInConfiguration]]
.Mit @configuration annotierte Klassen sollten keine Field Injection enthalten
[source,cypher,role=constraint,requiresConcepts="spring:Configuration,spring:InjectedField"]

[[structure:NoFieldInjectionInComponent]]
.Mit @component annotierte Klassen sollten keine Field Injection enthalten
[source,cypher,role=constraint,requiresConcepts="spring:Component,spring:InjectedField"]

Constraint: InjectablePackageProtected

[[spring:InjectablePackageProtected]]
.Injectables müssen package-protecte sein, wenn sie ein (nicht-Spring) Interface implementieren
[source,cypher,role=constraint,requiresConcepts="spring:Injectable,spring:BusinesInterface"]

Constraint: ComponentsMustNoImplementSpringInterface

[[structure:ComponentsMustNoImplementSpringInterface]]
.Application Componenten sollten keine Framework spezifischen interfaces implementieren
[source,cypher,role=constraint,requiresConcepts="spring:Component,spring:FrameworkInterface"]

Custom CustomizableTraceInterceptor reported as InjectablesMustNotBeInstantiated

Given I have:

@Configuration
public class LoggingInterceptorConfig {
    @Bean
    public CustomizableTraceInterceptor methodTraceLoggingInterceptor() {
        return new MethodTraceLoggingInterceptor();
    }
}

and

public class MethodTraceLoggingInterceptor extends CustomizableTraceInterceptor {

    MethodTraceLoggingInterceptor() {
        setEnterMessage("> [ENTRY] " + PLACEHOLDER_METHOD_NAME + "(" + PLACEHOLDER_ARGUMENTS + ")");
        setExitMessage("< [EXIT] " + PLACEHOLDER_METHOD_NAME + " @ " + PLACEHOLDER_INVOCATION_TIME + " ms: " + PLACEHOLDER_RETURN_VALUE);
        setExceptionMessage("< [THROW] " + PLACEHOLDER_METHOD_NAME + " : " + PLACEHOLDER_EXCEPTION);
        setUseDynamicLogger(true);
    }

    @Override
    protected boolean isLogEnabled(Log logger) {
        return logger.isDebugEnabled();
    }

    @Override
    protected void writeToLog(Log logger, String message, Throwable ex) {
        if (ex != null) {
            logger.warn(message, ex);
        } else {
            logger.debug(message);
        }
    }
}

it is reported as

[ERROR]   Type=foo.MethodTraceLoggingInterceptor, Method=foo.MethodTraceLoggingInterceptor#void <init>(), Injectable=org.springframework.aop.interceptor.CustomizableTraceInterceptor, LineNumber=8

even though I don't have any init method in MethodTraceLoggingInterceptor

Constraint: ImplementationDependencies

[[structure:ImplementationDependencies]]
.Ein Controller referenziert entweder einen Service oder das entsprechende Repository, niemals beides
[source,cypher,role=constraint,requiresConcepts="spring:Controller,spring:Service,spring:Repository"]

Constraint: ConstructorFieldsMustNotBeManipulated

[[structure:ConstructorFieldsMustNotBeManipulated]]
.Felder die vom Konstruktor gesetzt werden, dürfen durch keine andere Methode überschrieben werden
[source,cypher,role=constraint,requiresConcepts="spring:ConstructorField"]

Constraint: InjectablesMustNotBeInstantiated

[[spring:InjectablesMustNotBeInstantiated]]
.Newables dürfen keine Injectables erzeugen
[source,cypher,role=constraint,requiresConcepts="spring:Injectable"]

Group 'package:Default' is not defined. Concept package:Default not found.

When I copy & paste jqassistant-maven-plugin configuration from https://101.jqassistant.org/getting-started-spring-boot-maven/readme.html#MavenPlugin and run mvn clean install, I get [ERROR] Failed to execute goal com.buschmais.jqassistant:jqassistant-maven-plugin:1.4.0:analyze (default-cli) on project mer-web: Analysis failed.: Group 'package:Default' is not defined. Concept package:Default not found. -> [Help 1]

Why is that?

I have to comment out <group>package:Default</group>, so that it returns constraint violations.

Constraints should only apply to production code

Most of the constraints provided by the plugin only make sense for production code, therefore test code (i.e. test artifacts) should be ignored.

Idea: Parsing Spring Configurations

Making relationships to classes (beans) / modules and to find missing relationships

Move plugin to new org jqassistant-plugin

Spring boot configuration properties report as FieldsOfInjectablesMustNotBeManipulated

Given I have:

@ConfigurationProperties("security")
@Getter
@Setter
public class SecurityProperties {
    private boolean enabled = true;
}

When I run jqassistant-maven-plugin:1.4.0:analyze (default-cli), then it reports:

[ERROR] --[ Constraint Violation ]-----------------------------------------
[ERROR] Constraint: spring-injection:FieldsOfInjectablesMustNotBeManipulated
[ERROR] Severity: MAJOR
[ERROR] Fields of injectable types must not be manipulated, except from constructors.
[ERROR]   Message=...SecurityProperties.setEnabled(?) writes field 'enabled' at line 13, Injectable=...SecurityProperties, Method=...SecurityProperties#void setEnabled(boolean), Field=...SecurityProperties#boolean enabled, LineNumber=13

The reason I have setters is missing support for immutable @ConfigurationProperties.

spring-component:*MustOnlyDependOn* should report violations not aggregated

The following three constraints report violations as a a collected list. However, this is impractical as tracking changes (both positive and negative) is made more difficult as the rule "one row - one violation" doesn't hold anymore Each dependency shoud be considered as a violation.

• spring-component:ControllerMustOnlyDependOnServicesRepositories
• spring-component:ServiceMustOnlyDependOnServicesRepositories
• spring-component:RepositoryMustOnlyDependOnRepositories

So the following return statement:
RETURN repository as Repository, collect(other) as InvalidDependencies
should be changed to
RETURN repository as Repository, other as InvalidDependency

Spock specifications not treated as tests

Given I have:

@SpringBootTest
class MerchantImportExportSpec extends spock.lang.Specification {
    @Autowired
    MerchantService merchantService
    ...
}

plugin reports it as [ERROR] Message=null, Injectable=merchant.MerchantImportExportSpec, Method=merchant.MerchantImportExportSpec#void setMerchantService(merchant.MerchantService), Field=merchant.MerchantImportExportSpec#merchant.MerchantService merchantService, LineNumber=null

This is test, so I'd expect I can inject using @Autowired .
Is it possible to tweak the rule so that it treats Spock specifications as tests as well?

"spring-injection:InjectablesMustNotBeAccessedStatically" should report containing type

The constraint reports the method and the field but from the Asciidoc report the containing type is not visible and should be added

Use Github Actions to provide a simple CI/CD Buildchain

As core developer of jQAssistant, I want this project to be build on various plattforms with different JDK versions on each commit, so that I know if the change done my the commit does not break the build and the software is fully functional.

The following acceptance criterias must be met:

• mvn -DskipTests -Djqassistant.skip clean install must pass
• mvn -Djqassistant.skip clean install must pass
• mvn -P IT clean install must pass
• mvn -DskipTests -Djqassistant.skip=false clean install must pass

[spring-injection:InjectablesMustNotBeInstantiated] Avoid Cartesian product. Ignore Generated types in call chain.

Bug Description

Constraint spring-injection:InjectablesMustNotBeInstantiated builds Cartesian product when evaluating.

Constraint is violated if constructor of Injectable is called in call chain. If a Generated type is involved in the call chain, rule should not trigger.

Unify the Asciidoctor markup

Go over all documents of the documentation and unify the Asciidoctor markup.

New Constraint: JDK classes must not be injectables

Feature Description

It is possible to implement a Bean Producer that returns a JDK class.
With the concept spring-injection:BeanProducer, those return types will be marked as :Spring:Injectable.

In my opinion it is an anti-pattern to have Bean Producers return JDK classes and not use-case specific (custom) implementations.
Besides that, following rules will lead to lots of false-positives since JDK classes which are marked as Injectables will of course be used across the code in a different context.

• spring-injection:InjectablesMustNotBeInstantiated
• spring-injection:InjectablesMustNotBeHeldInStaticVariables
• spring-injection:InjectablesMustNotBeAccessedStatically

For that I propose to add a new constraint spring-injection:JdkClassesMustNotBeInjectables

How to Test

Create a bean producer returning any JDK class. If the constraint fails, the ticket is done.

Definition of Done for the Implementers

• We got a final feedback from the reporting user (if applicable)
• Unittests have been written (if applicable)
• Integration tests have been written (if applicable)
• Test coverage is the same or even better then before (if applicable)
• Documentation has been written (if applicable)
• Added a note on the new feature to the release notes (if applicable)

Migrate the Plugin Documentation of the Spring Plugin to the new Reference Manual

Task Description

Migrate the existing documentation of the Spring Plugin to the new reference manul.

Definition of Done for the Implementers

• The reference manual contains the same information as the old user manual
• The section on the Spring Plugin in the old user manual points to the section for the plugin in the reference manual

Custom annotations with @RestController or @Component are getting wrong labels.

I have a custom annotation annotated with @RestController

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@RestController
public @interface MyCustomAnnotation {
}

which gets labelled with Spring, RestController, Controller, Component, Injectable by the spring-mvc:RestController concept. This leads to false positives with the spring-component:ControllerMustOnlyDependOnServicesRepositories constraint as annotation class is recognised as a controller, but actually is not.

spring-mvc:RestController concept should exclude annotations from being labelled. The same applies for the following concepts: spring-component:Controller, spring-component:Service, spring-component:Component, spring-component:Configuration, spring-data:AnnotatedRepository.

Current workaround is the following concept:

  <concept id="project:spring:AnnotationAnnotatedWithRestController">
    <providesConcept refId="spring-component:Component"/>
    <providesConcept refId="spring-component:Controller"/>
    <providesConcept refId="spring-injection:Injectable"/>
    <requiresConcept refId="spring-mvc:RestController"/>
    <description>
      Removes all Spring labels from annotations annotated with @RestController.
    </description>
    <cypher><![CDATA[
      MATCH (annotation:Annotation)-[:ANNOTATED_BY]->()-[:OF_TYPE]->(annotationType:Type)
      WHERE annotationType.fqn = "org.springframework.web.bind.annotation.RestController"
     REMOVE annotation:Spring:RestController:Controller:Component:Injectable
     RETURN annotation as Annotation
    ]]></cypher>
  </concept>

IT tests are not executed with -PIT

When bulding the project with mvn clean install -PIT, no IT tests are executed (the same can be seen in the build logs of Jenkins). Example log:

[INFO] Running com.buschmais.jqassistant.plugin.spring.test.constraint.SpringInterfacesIT
[main] INFO com.buschmais.jqassistant.core.plugin.impl.PluginConfigurationReaderImpl - Scanning for jQAssistant plugins...
[main] INFO com.buschmais.jqassistant.core.plugin.impl.PluginConfigurationReaderImpl - [Common, Common Test, Core Analysis, Core Report, Java, Spring, XML].
[INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.328 s - in com.buschmais.jqassistant.plugin.spring.test.constraint.SpringInterfacesIT

spring-boot:Default causes false positives with dependency:PackageCycles

Scenario

In my project, I'm using spring-boot:Default in combination with dependency:PackageCycles.

The analyzer reports a package cycle between two packages service and config that actually do not have a cycle.

Analysis

Package service does depend on package config. A class in package config autowires List<FooService> where all beans implementing FooService are located in Package service. These beans are treated as a virtual dependency which results in the inverse relation

config->[d:DEPENDS_ON]->service with d.virtual = true

Since the PackageCycles rule is not aware of the virtual flag, it will report a cycle between service and config.

Proposal

Express virtual dependencies by a separate relation.

Controller annotated with @RestController is not view as Spring controller

org.springframework.web.bind.annotation.RestController annotation is itself annotated with @Controller

Constraint: ApplicationMustBeInBasePackage

[[structure:ApplicationMustBeInBasePackage]]
.SpringApplication Klasse muss im Basispackage liegen
[source,cypher,role=constraint,requiresConcepts="spring-boot:Application"]

Add rule to recommend removing @Autowired from a canonical constructor

If a Spring bean exposes a canonical constructor, that constructor does not need to be annotated with an injection annotation like @Autowired or @Inject. We could report that the annotation can be removed.

With my rudimentary Cypher skills I came up with this one but it looks like I can't count over the occurrences of constructors like this.

MATCH
  (injectable:Injectable)-[:DECLARES]->(constructor:Constructor),
  (constructor:Constructor)-[:ANNOTATED_BY]->(annotation:Annotation)
WHERE
  count(constructor) > 1 AND annotation.fqn in [
    "org.springframework.beans.factory.annotation.Autowired",
    "javax.inject.Inject"
  ]
RETURN
  "Unique constructor of type " + injectable.fqn + " does not need to be annotated with " + annotation.fqn as Message,
  injectable as Injectable, constructor as Constructor, annotation as Annotation

Groovy class is reported as FieldsOfInjectablesMustNotBeManipulated

When I create groovy class which is injectable, plugin reports FieldsOfInjectablesMustNotBeManipulated because of some getMetaClass() and setMetaClass() which is not in my control

[ERROR]   Message=null, Injectable=foo.WiremockLauncher$WiremockRunner, Method=foo.WiremockLauncher$WiremockRunner#groovy.lang.MetaClass getMetaClass(), Field=foo.WiremockLauncher$WiremockRunner#groovy.lang.MetaClass metaClass, LineNumber=null

[ERROR]   Message=null, Injectable=foo.WiremockLauncher$WiremockRunner, Method=foo.WiremockLauncher$WiremockRunner#void setMetaClass(groovy.lang.MetaClass), Field=foo.WiremockLauncher$WiremockRunner#groovy.lang.MetaClass metaClass, LineNumber=