Skip to content

Commit

Permalink
Fix AspectJ basics (#137)
Browse files Browse the repository at this point in the history
* Centrally version-manage JUnit 4.13.2, also for tests

ITs now use the same version as the project itself, no longer 3.8.2.

* Bump AspectJ to 1.9.7.M3, supporting Java 15+16

Also pull up the version property into the parent POM, in order to be
able to use it in ITs, too, similarly to the ErrorProne version.

* Fix basic AspectJ compiler functionality + add integration test

This is a first, basic step into improving AspectJ support for Plexus
Compiler. It looks like nobody ever noticed, that target and release
parameters are not forwarded to AJC, resulting in all class files being
compiled to Java 1.2 target. I.e. that annotation-based @AspectJ syntax
was completely unsupported before. Since Plexus was introduced 15 years
ago, nobody ever did anything to upgrade the functionality a bit, only
AspectJ versions were bumped before.

Furthermore, only *.java files were being considered for compilation, no
*.aj files. This was also fixed, but at the cost of duplication with
regard to redefining two static methods 'getSourceFiles' and
'getSourceFilesForSourceRoot'. Other compiler components did it
similarly, so the necessary refactoring to make the methods non-static
and break them down into smaller parts in order to be able to override
only the parts of them dealing with source file extensions, is a TODO
which involves refactoring the other compiler components, too. I did not
do that in this first step.

Another TODO: In contrast to the ECJ adapter, which uses the batch
compiler, AJC (an ECJ fork!) is used via the internal AJDT interface,
which is designed to be used by the Eclipse IDE. It would have been
easier to actually also use the batch compiler right from the start,
because then we can more easily map command line parameters and keep the
adapter layer thin. Why the original author did that in 2005, remains a
mystery.

The new AspectJ integration test uses a mixture of
  - native and @AspectJ syntax variants,
  - *.java and *.aj aspect source files,
  - production (src/main) and test (src/test) aspects.
This is all covered by a single IT. In the end, the test asserts on the
complete JUnit console log, which has to look a certain way in order to
reflect that
  - compilation and test were successful,
  - all 3 aspects did what they are supposed to do.

* Activate 'streamLogsOnFailures' for Maven Invoker

This way, it is easier to debug failing integration tests on CI
environments, where it is difficult or impossible to add the individual
IT build logs.

OTOH, the Maven debug statements in the test output are usually making
it quite hard to find the actual error that occurred. While helpful in
rare cases, most of the time the normal Maven build output should easily
suffice in order to identify and fix integration test problems.
Therefore, I deactivated 'debug' for Maven Invoker. This should strike a
sensible balance between inlining IT error logs into the outer Maven
build log in case of errors and not polluting it with too much
information.
  • Loading branch information
kriegaex committed Jun 16, 2021
1 parent bfd5875 commit a007765
Show file tree
Hide file tree
Showing 16 changed files with 351 additions and 93 deletions.
3 changes: 2 additions & 1 deletion plexus-compiler-its/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
<goal>verify</goal>
</goals>
<configuration>
<debug>true</debug>
<debug>false</debug>
<streamLogsOnFailures>true</streamLogsOnFailures>
<projectsDirectory>src/main/it</projectsDirectory>
<cloneProjectsTo>${project.build.directory}/it</cloneProjectsTo>
<postBuildHookScript>verify</postBuildHookScript>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

invoker.java.version = 1.8+
invoker.goals = clean test
#invoker.buildResult = failure
73 changes: 73 additions & 0 deletions plexus-compiler-its/src/main/it/aspectj-compiler/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Licensed to the Apache Software Foundation (ASF) under one
~ or more contributor license agreements. See the NOTICE file
~ distributed with this work for additional information
~ regarding copyright ownership. The ASF licenses this file
~ to you under the Apache License, Version 2.0 (the
~ "License"); you may not use this file except in compliance
~ with the License. You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing,
~ software distributed under the License is distributed on an
~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
~ KIND, either express or implied. See the License for the
~ specific language governing permissions and limitations
~ under the License.
-->

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>org.apache.maven.plugins.compiler.it</groupId>
<artifactId>aspectj-compiler</artifactId>
<version>1.0-SNAPSHOT</version>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
</properties>

<build>
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.8.1</version>
<configuration>
<compilerId>aspectj</compilerId>
</configuration>
<dependencies>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-compiler-api</artifactId>
<version>@pom.version@</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-compiler-aspectj</artifactId>
<version>@pom.version@</version>
</dependency>
</dependencies>
</plugin>
</plugins>
</build>

<dependencies>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>@junit.version@</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.aspectj</groupId>
<artifactId>aspectjrt</artifactId>
<version>@aspectj.version@</version>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.acme;

public class Application {
public static void main(String[] args) {
System.out.println("Running application");
new Application().greet(args[0]);
}

public String greet(String name) {
return "Hello " + name;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.acme;

import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;

@Aspect
public class MyAnnotationDrivenAspect {
@Before("execution(static * *(..)) && within(Application)")
public void myAdvice(JoinPoint joinPoint) {
System.out.println(joinPoint);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.acme;

public aspect MyNativeAspect {
before() : execution(!static * *(..)) && within(Application) {
System.out.println(thisJoinPoint);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
The Apache Maven Compiler (MC) API only allows a single source file extension, such as '.java'.
The AspectJ Compiler (AJC) should consider two default extensions, though: '.java' and '.aj'.
In order to achieve that, the Plexus AJC component tells MC to give it all files,
subsequently filtering for those two extensions.

The purpose of this file is to make sure that even though MC finds it in the source folder,
Plexus AJC filters it out and AJC does not try to compile it.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.acme;

import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

public class ApplicationTest {
@Test
public void testGreet() {
assertEquals("Hello Jane", new Application().greet("Jane"));
}

@Test
public void testMain() {
Application.main(new String[] { "Joe" });
assertTrue(true);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.acme;

import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;

@Aspect
public class TestAspect {
@Before("call(* *(..)) && !within(TestAspect)")
public void beforeCall(JoinPoint joinPoint) {
System.out.println(joinPoint);
}
}
34 changes: 34 additions & 0 deletions plexus-compiler-its/src/main/it/aspectj-compiler/verify.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
def logFile = new File( basedir, 'build.log' )
assert logFile.exists()
content = logFile.text.normalize()

def junitLog = """Running org.acme.ApplicationTest
call(String org.acme.Application.greet(String))
execution(String org.acme.Application.greet(String))
call(void org.junit.Assert.assertEquals(Object, Object))
call(void org.acme.Application.main(String[]))
execution(void org.acme.Application.main(String[]))
Running application
execution(String org.acme.Application.greet(String))
call(void org.junit.Assert.assertTrue(boolean))
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0""".normalize()

assert content.contains( junitLog )
3 changes: 2 additions & 1 deletion plexus-compiler-its/src/main/it/error-prone-compiler/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>3.8.2</version>
<version>@junit.version@</version>
<scope>test</scope>
</dependency>
</dependencies>

Expand Down
3 changes: 2 additions & 1 deletion plexus-compiler-its/src/main/it/simple-javac/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>3.8.2</version>
<version>@junit.version@</version>
<scope>test</scope>
</dependency>
</dependencies>

Expand Down
4 changes: 0 additions & 4 deletions plexus-compilers/plexus-compiler-aspectj/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@
<name>Plexus AspectJ Compiler</name>
<description>AspectJ Compiler support for Plexus Compiler component.</description>

<properties>
<aspectj.version>1.9.6</aspectj.version>
</properties>

<dependencies>
<dependency>
<groupId>org.codehaus.plexus</groupId>
Expand Down
Loading

0 comments on commit a007765

Please sign in to comment.