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

Support aspectj-maven-plugin using AspectJ idea plugin #7

Closed
grisha9 opened this issue Mar 28, 2024 · 20 comments
Closed

Support aspectj-maven-plugin using AspectJ idea plugin #7

grisha9 opened this issue Mar 28, 2024 · 20 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@grisha9
Copy link
Owner

grisha9 commented Mar 28, 2024

If module contain aspectj-maven-plugin and classpath has library org.aspectj:aspectjtools and AspectJ idea plugin installed then GMaven on import try to configure java compiler to use "ajc" compiler (File | Settings | Build, Execution, Deployment | Compiler | Java Compiler)
Link to AspectJ plugin - https://plugins.jetbrains.com/plugin/4679-aspectj

@grisha9 grisha9 added the enhancement New feature or request label Mar 29, 2024
@grisha9
Copy link
Owner Author

grisha9 commented Mar 29, 2024

An example of how this would work:

simplescreenrecorder-2024-03-29_09.33.09.mp4

@grisha9
Copy link
Owner Author

grisha9 commented Mar 29, 2024

Preview available in 233.25.1 version from beta repo https://plugins.jetbrains.com/plugin/22370-gmaven/versions/beta

@grisha9 grisha9 self-assigned this Mar 29, 2024
@kriegaex
Copy link

kriegaex commented Mar 29, 2024

If module contain aspectj-maven-plugin and classpath has library org.aspectj:aspectjtools

Please note that aspectjtools does not belong on the module classpath, only on the plugin classpath for AJMaven. The fact that it is on the classpath in the sample project you are using has other reasons, namely that it the library is to be packaged into AJDT for Eclipse.

What you need to do is resolve aspectjtools as seen by the plugin. The plugin has a default dependency on a certain aspectjtools version, but users can override that plugin dependency via plugin configuration, which is common practice. I.e., you rather need to resolve the plugin classpath, not the module classpath.

Please also note that, despite the fact that the AspectJ team endorses dev.aspectj:aspectj-maven-plugin, the older and less feature-rich org.codehaus.mojo:aspectj-maven-plugin is also still popular. Both group IDs are also supported by IDEA out of the box. The AspectJ.dev variant has more options than the Mojohaus one, but otherwise they are pretty much identical. Edit: Oh OK, in your source code I see that you do not check the group ID at all, i.e. you will automatically match both.

@kriegaex
Copy link

kriegaex commented Mar 29, 2024

IDEAJust quickly tried the beta in a regular Maven project != AJDT. Like I thought, aspectjtools is not detected and Javac is selected as Java compiler, not Ajc. After manually switching to Ajc, it looks like this:

image

When using IDEA's Maven integration, it looks like that:

image

Furthermore, plugin options for the compiler are not imported, as it seems. I.e., plugin option <enablePreview>true</enablePreview> (or equivalently, <compilerArgs>--enable-preview</compilerArgs>) is ignored, which I noticed by chance when compiling some Java 22 code with preview features. Again, with the built-in IDEA Maven plugin it works.

GMaven:
image

IDEA built-in:
image

@kriegaex
Copy link

Being the maintainer of both AspectJ and AspectJ.dev AJ Maven, feel free to connect with me, so I can explain all the plugin options to you. The AspectJ compiler is not just a drop-in replacement for Javac or ECJ, it has more options that need to be passed through from AJ Maven to aspectjtools when compiling from IDEA.

@grisha9
Copy link
Owner Author

grisha9 commented Mar 29, 2024

Thanks for your feedback. For now this is just a preview to show what the aspect integration would look like through the AspectJ IDEA plugin on example ajdt project(which works thanks to the "intellij" profile). In the final version, it will be necessary to support the maven plugins configurations from both AspectJ and AspectJ.dev AJ Maven.

@kriegaex
Copy link

Sure. I simply wanted to provide timely feedback while still being in the mental and technical context of this issue. Otherwise, I might have forgotten about it and next time might have needed to start over. Being an agile coach, I do believe in the power of short feedback cycles. 😉

@grisha9
Copy link
Owner Author

grisha9 commented Apr 9, 2024

Preview available in 233.25.2 version from beta repo https://plugins.jetbrains.com/plugin/22370-gmaven/versions/beta

@kriegaex
Copy link

I just installed the plugin version and tried it on a simple AspectJ Maven project. It seems to have absolutely no effect. Neither is there any AspectJ facet in the imported project, nor is AspectJ compilation taking place in the IDE build.

@grisha9
Copy link
Owner Author

grisha9 commented Apr 11, 2024

My plugin does not create facets. Because this is native functionality, which is a closed part of the aspetJ plugin for IDEA Ultimate. And therefore they only work in conjunction with the bundled Maven plugin. And I can’t make my free plugin depend on a paid IDEA Ultimate aspectJ plugin.
My plugin only adds compiler settings. For ajdt parent project it is look this on my PC:
image

If this does not work for you, please attach the build files of the your test project, I will check it.

@grisha9
Copy link
Owner Author

grisha9 commented Apr 12, 2024

I found a potential problem. I don't resolve aspectj-maven-plugin dependencies. And if the required dependency is not in the local repository, then I will not find it and will not configure the compiler settings. I fixed this in version 233.25.3.
I checked it on the project from your examples.
And also added a unit test.
And in the IDE itself, this example works correctly for me.

image

@kriegaex
Copy link

kriegaex commented Apr 13, 2024

It is still not working. Obviously, the import does not work correctly, if both Maven Compiler and AJ Maven are specified in the POM. That is a problem. If I uncomment Maven Compiler, the import works. But still, some options are imported incorrectly, e.g. <encoding>UTF-8</encoding> leads to error ajc: unrecognized single argument: "-encoding:UTF-8":

image

One more issue is that after manual or automatic GMaven import, no re-build is issued even after the configuration has changed. I have to manually rebuild the project for the config changes to become effective.

@grisha9
Copy link
Owner Author

grisha9 commented Apr 13, 2024

Obviously, the import does not work correctly, if both Maven Compiler and AJ Maven are specified in the POM

Thank you for helping to localize the problem

leads to error ajc: unrecognized single argument: "-encoding:UTF-8"

How to correctly pass parameters? Parameter Xlint and parameter encoding are of type string.
Xlint must be passed as -Xlint:ignore
But encoding must be passed as -encoding UTF-8
I don’t understand from the documentation how to correctly pass string parameters. Can you tell me how to do it correctly?

no re-build is issued even after the configuration has changed

First I will need to figure out how this works in the bundled maven plugin. I think it's better to do this in another issue

@kriegaex
Copy link

Most AJC parameters are from upstream ECJ. Therefore, the syntax is like in the Eclipse Java Compiler. I only have influence on the AJC-specific ones.

Simply run ajc -? and ajc -X to see all the options after having installed AspectJ by running the installer from an admin console, e.g. java -jar aspectj-1.9.22.jar. The installer can be downloaded from https://github.com/eclipse-aspectj/aspectj/releases/tag/V1_9_22. Or simply start the compiler from an existing aspectjtools-x.y.z.jar via java -cp aspectjtools.jar org.aspectj.tools.ajc.Main -?.

For your convenience, there is also documentation here:

@grisha9
Copy link
Owner Author

grisha9 commented Apr 14, 2024

I fixed compiler priority and string params at version 233.25.4.

@kriegaex
Copy link

Thanks, @grisha9. Now it works. As for compiler preference, this is exactly the weakness in IDEA: The Java compiler can either be Ajc or Javac (or something else like GrEclipse), but not both, even if Maven defines multiple plugins and executions, Maven Compiler doing annotation processing only, AJ Maven Java and AspectJ compilation only or some mix. A perfect IDE build would be able to compile multiple times with multiple compilers (e.g. also test vs application code) in the same order as Maven.

@grisha9
Copy link
Owner Author

grisha9 commented Apr 14, 2024

Thank you for the clarification. I think this is another potential problem with my implementation when I simply replace one compiler with another. Then you may need to take the annotation processor settings from maven-compiler-plugin. IDEA has separate settings for the annotation processor

@grisha9 grisha9 modified the milestones: 232.11, 233.26 Apr 14, 2024
@kriegaex
Copy link

kriegaex commented Apr 15, 2024

The annotation processor is just an example. You see, if you really want to cover all important cases in a Maven plugin, your initial goal to avoid adding logic from the plugins or from Maven to the plugin itself does not work. At the end of a day, that is exactly what you are going to do if you want it to work in more than just pareto-style 80-to-20 cases. Somehow, the Maven semantics need to be reflected in the plugin mapping those settings to IDEA configurations. But even that is limited by what IDEA offers. I do not envy you in this regard. But OTOH, I do think that given JetBrains' slow adoption rate of change requests and bug reports, there is room for your plugin, if it solves problems that the built-in one does not. This is the main reason I am testing (albeit with just very small time slices) and providing feedback.

@grisha9
Copy link
Owner Author

grisha9 commented Apr 15, 2024

Thank you. Yes, my main goal is to import a project model from Maven into the IDE. There are nuances to assembling and launching a project from IDEA. I have plans to also delegate some of this to Maven - for example, running tests.

@grisha9
Copy link
Owner Author

grisha9 commented Apr 15, 2024

fixed in 233.26

@grisha9 grisha9 closed this as completed Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants