Before, we used 1.9.7.BUILD-SNAPSHOT, which according to Andy Clement
was originally an intent across a group of Spring projects he was
involved in, to ensure that SNAPSHOTS were sorted alphabetically ahead
of MILESTONEs and ahead of RCs.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
This involves replacing references in weaver application code as well as
a few tests.
In order to make AspectJ weaver + tools contain a relocated ASM version,
I added a Maven Shade relocation step after Maven Assembly created the
uber JARs. Relocation works for both binaries and sources and also
encompasses Class::forName calls like in class AsmDetector.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Upon special request by Andy Clement, I included 'lib' as a child module
in the parent POM again, making several modules which refer to
downloaded library files dependent the 'lib' module. I am not sure I
caught all of them, but I hope so.
Now after cloning the project and configuring the token for reading from
GitHub Packages (sorry!), you can just run a Maven build for the main
project and no longer need to fail the first build, read the Maven
Enforcer message and run 'cd lib && mvn compile' as a first step. This
convenience comes at the price of a more complex POM and two new
profiles:
- Profile 'provision-libs' is auto-activated by the absence of a
marker file, kicking off the library provisioning process and
creating same marker file at the end, if successful. Therefore,
during subsequent builds libraries will not be re-provisioned,
because the marker file exists and Maven skips all download and
(un)zip steps, which saves build time and bandwidth. Otherwise
offline builds would not work either.
- Profile 'clean-libs' needs to be activated manually, because by
default 'mvn clean' will not erase provisioned libraries. In most
cases, even after a clean a developer does not want to re-provision
all libraries if they have not changed (e.g. new JDT Core build).
But if you do wish too erase the libraries and the marker file, just
call 'cd lib && mvn -P clean-libs clean'.
Please note: The Maven Enforcer build step, which additionally checks
for existence of other files, still exists and was moved from the parent
POM to 'libs'. No matter if provisioning was just done or skipped
because the main marker file exists, a quick heuristic check for that
list of files is done during each build, failing the build with a
comprehensive message if an inconsistency was found. The error message
says which files are missing and tells the user:
"There is an inconsistency in module subdirectory 'lib'. Please run
'mvn --projects lib -P clean-libs clean compile'. This should take
care of cleaning and freshly downloading all necessary libraries to
that directory, where some tests expect them to be."
This should cover the topic.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Remove redundant 'name' and 'packaging' tags from POMs
If 'name' is identical to 'artifactId' and 'packaging' has the default
value 'jar', we can just remove those tags from the POM.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Clean up Maven dependencies using 'dependency:analyze' goal
Notably, this change involves a partial revert of @4a5660b3, because we
are not using JUnit Jupiter yet but still JUnit 4 tests. See discussion
under commit at https://github.com/eclipse/org.aspectj/commit/4a5660b3.
Many other warnings - concerning both used undeclared and unused
declared dependencies - were eliminated by adding or removing the
corresponding dependencies from the POMs. Furthermore, I tried to make
sure that some clearly test-scoped dependencies are now actually
declared as such, so as to avoid unwanted transitivity bleeding into
compile scope and maybe unwanted classes ending up in uber JARs via
Maven Shade or Maven Assembly.
TODO: I am not so sure why modules other than 'run-all-unit-tests' would
depend on test JARs. I hope I broke nothing essential there. As of
today, the other modules where I found '<type>test-jar</type>'
dependencies are:
- ajde
- testing
- testing-drivers
- tests
- weaver
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Replace JDT Core system dependency by deployed one
Get rid of system paths. Instead, rely on JDT Core Shadows to deploy
both binary and source JARs to GitHub Packages. The former module
directory was deleted completely. Instead, the JARs are redundantly
copied into 'libs/jdtcore-aj' in order to be found there by tests and
other Ant scripts.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Replace system-scoped dependency on commons by granular dependencies
There are only two direct dependencies used in AspectJ code:
- Commons Digester (module 'testing')
- Commons Logging (module 'org.aspectj.matcher')
I declared those two and experimentally removed all the other
system-scoped dependencies, as it should be. Let's see if the build
works with transitive dependencies.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Make all tests run on Java 16 via '-add-opens' JVM option
Due to JEP 260 (Encapsulate Most Internal APIs), aspect weaving on
Java 16 now requires '--add-opens java.base/java.lang=ALL-UNNAMED' on
the command line. Otherwise there will be illegal access exceptions for
some internal API calls AspectJ needs, most prominently when trying to
define classes in other packages or modules.
This had to be done on several levels:
- Maven Surefire: running tests in a JVM directly forked by Surefire.
In order to make this backwards compatible, I added two profiles
with JDK-level-dependent auto-activation, one 8-15 and one 16+. In
the latter a property containing the JVM parameter is defined, in
the former it is empty, i.e. the JVM is started without the
parameter. In Java 8 the parameter did not even exist, in Java 9+ we
could use it, but we need to test how users use AspectJ.
- RunSpec: Whenever an XML test is declared to use '<run>', we need to
determine the current JVM version and again dynamically add the
parameter when forking the target JVM.
- AntSpec: Whenever an XML test is declared to use '<ant>', we need to
determine the current JVM version dynamically add two properties
usable from within Ant scripts: 'aj.addOpensKey' and
'aj.addOpensValue'. Unfortunately, Ant needs to use two '<argLine>'
parameters, because the two parts of the option are separated by a
space character.
- Ant scripts: When triggered by an AntSpec, each Ant target using LTW
needs to manually set
<jvmarg value="${aj.addOpensKey}"/>
<jvmarg value="${aj.addOpensValue}"/>
for each '<java>' task. It was quite tedious to find all(?) of them.
TODO: In the AspectJ 1.9.7 release notes we need to document that this
parameter is now needed for LTW.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
After Ajc usage text output is filtered into its own category
IMessage.USAGE now - see commit @31b2d60b - some tests in module
'org.aspectj.ajdt.core' were failing. I fixed and also improved them a
bit.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Add Java 16 test suite for AspectJ 1.9.7 + test refactorings
- Test all features which were preview in 14+15 and are now final in 16,
compiling them with language level 16.
- For Java 15 we only have sanity tests (and of course the Java <14
tests), compiling Java 16 features to target 15 does not seem to work.
- Test remaining Java 16 preview feature (sealed classes).
- Instead of overriding runTest(String) in several base classes like
XMLBasedAjcTestCaseForJava*Only or XMLBasedAjcTestCaseForJava*OrLater,
we now override setUp() from JUnit's TestCase base class. This will
run before runTest(String) and make the tests fail much faster, if a
user tries to run them on the wrong VM.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Fix test relying on JVM warning being stripped from output
The line in which warnings like "Archived non-system classes are
disabled because the java.system.class.loader property is specified"
appears can start with e.g."OpenJDK 64-Bit Server VM" or "Java
HotSpot(TM) 64-Bit Server VM". Therefore, an exact match on the former
worked on Linux, but not on Windows, or maybe the difference is
generally between Oracle and OpenJDK. anyway, I use Oracle on Windows
and my build failed. Now it is fixed because I made the match more
generic using a regex.
I also removed a now obsolete check for the occurrence of the stripped
line in test "JDK14 LTW with XML".
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Improve usage text, error and warning output in batch compiler
- Usage texts are now printed to stdOut, no longer stdErr.
- 'java ...Main -?' no longer prints usage text twice (once to stdOut
and then again to stdErr).
- AjdtCommand.inferKind: Usage texts are no longer mis-identified as
warnings or errors just because they contain substrings "warning" or
"error". Matching is now more precise, looking for "[warning]" and
"[error]". But in that case the method would not be called anyway
because errors and warnings are identified in other ways already. As a
fall-back, the categories IMessage.ERROR and IMessage.WARNING still
exist in the method.
- In case of compile errors, no usage message is printed anymore,
because previously the user had to scroll up a lot in order to see the
actual messages. This is also in line with ECJ. The same is true for
warnings, but it was like this in Ajc already.
- AjdtCommand.inferKind: There is a new category IMessage.USAGE
especially for AspectJ usage texts, which will be identified by string
matching and then correctly handled (i.e. printed to stdOut, not
stdErr).
- Usage text printing is no longer done in AspectJ but in the AspectJ
"shadows" fork of JDT. This helps to get rid of some now obsolete code
here.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Do not run tests using Java 14 preview features on this Java 15 compiler
These tests need a Java 14 level AspectJ compiler, because they use
version-specific preview features. This compiler has been upgraded
to a Java 15 compliant JDT Core already, i.e. it does not support
preview features of a previous version anymore.
An error message similar to the above explanation will appear when
trying to run any XMLBasedAjcTestCaseForJava14Only subclass, such as
Ajc196PreviewFeaturesTests (currently the only one).
When running AllTestsAspectJ196, Ajc196PreviewFeaturesTests will not be
added to the test suite anymore.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Fix outdated ajcTestSuite.dtd and lots of XML problems
Now there should be no more inspection warnings when working with XML
test definitions. Only the strangely looking XML files used by
PureJavaTests and KnownLimitationsTests are left to be analysed and
fixed.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Make sure Java 14-only tests using preview are not executed on JVM 15+
Some Ajc196 tests are using preview features (see .../ajc196.xml), i.e.
they will fail on Java 15+ because code compiled with '--enable-preview'
can only run on the same JVM version, not on a more recent one. Hence,
the preview-using tests are now being excluded in order to make the
build run on Java 15, even though no Java 15 features are present in the
current 1.9.7 snapshot.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Improve text matching in OutputSpec, fixing some failing Windows tests
Some Java 14 text block tests failed on Windows because a
StringTokenizer was used to split by LF, but the Windows line
separator is CR+LF. Because a multi-line string ending with CR+LF is
printed via 'System.out.println' in the test code, another CR+LF is
added to the output, resulting in trailing CR+LF+CR+LF. Hence, between
the two LFs, the tokenizer actually found an additional line consisting
of CR (only on Windows, of course). Despite each line token actually
containing a trailing CR token, that did not matter much because
'String.trim' was used everywhere before comparing values.
Anyway, the improved OutputSpec uses text.trim().split("\\s*\n\\s*"),
which takes care of leading/trailing whitespace both around the whole
output and for each separate line.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Fix: WeaveSpec no longer ignores the '-Xlintfile' parameter
Some tests in ajc150.xml and ajc190_from150.xml contain '<weave ...>'
build steps with 'xlintfile="..."' parameters. Those parameters were
passed through to Java and aspect code building steps, but not to the
final weaving step, sometimes leading to spurious "type not exposed to
weaver" warnings which occurred for some local and CI builds, but not
always. Very strange indeed. Anyway, by making method
WeaveSpec.buildWeaveArgs() pass on this parameter, the tests seem to run
reliably now.
TODO: Why does Ajc report that warning if the application JAR is on the
inpath and the aspect JAR is on the aspectpath? Is it because a marker
annotation is defined within the aspect JAR? But actually, that should
not matter, especially not work once and fail at other times. I guess
there is a class loading order problem or similar involved.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Replace directory separator '/' and surrogate path separator ',' by
platform-specific separators File.separatorChar and
File.pathSeparatorChar, respectively. Also make sure that replacement
occurs during write access, not read access. This was handled
differently in both sibling classes.
I am not sure if that helps to fix any Linux CI tests, but it is worth a
try.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Reports on declarations of Collection variables made by using the collection class as the type, rather than an appropriate interface.
Signed-off-by: Lars Grefer <eclipse@larsgrefer.de>
Reports Collection.addAll() and Map.putAll() calls after instantiation of a collection using a constructor call without arguments. Such constructs can be replaced with a single call to a parametrized constructor which simplifies code. Also for some collections the replacement might be more performant.
Signed-off-by: Lars Grefer <eclipse@larsgrefer.de>
There are two styles to convert a collection to an array: either using a pre-sized array (like c.toArray(new String[c.size()])) or using an empty array (like c.toArray(new String[0]).
In older Java versions using pre-sized array was recommended, as the reflection call which is necessary to create an array of proper size was quite slow. However since late updates of OpenJDK 6 this call was intrinsified, making the performance of the empty array version the same and sometimes even better, compared to the pre-sized version. Also passing pre-sized array is dangerous for a concurrent or synchronized collection as a data race is possible between the size and toArray call which may result in extra nulls at the end of the array, if the collection was concurrently shrunk during the operation.
Signed-off-by: Lars Grefer <eclipse@larsgrefer.de>
Collections.sort() can be replaced with List.sort()
Reports calls to Collections.sort(list, comparator) which could be replaced with list.sort(comparator).
Signed-off-by: Lars Grefer <eclipse@larsgrefer.de>
Reports the copying of array contents to a collection where each element is added individually using a for loop. Such constructs may be replaced by a call to Collection.addAll(Arrays.asList()) or Collections.addAll().
Signed-off-by: Lars Grefer <eclipse@larsgrefer.de>
Reports "unboxing", e.g. explicit unwrapping of wrapped primitive values. Unboxing is unnecessary under Java 5 and newer, and can be safely removed.
Signed-off-by: Lars Grefer <eclipse@larsgrefer.de>
Alexander Kriegisch
Andy Clement
Lars Grefer