On JDK 21, generated code should apply "this-escape" warning suppression

[cowwoc]

Expected behavior

Ability to compile the generated classes on JDK 21 without any warnings. Our company uses the -Werror javac option so warnings break our build.

Actual behavior

When compiling, javac warns possible 'this' escape before subclass is fully initialized on this code:

public final TableField<Record, Long> ID = createField(DSL.name("id"), SQLDataType.BIGINT.nullable(false).identity(true), this, 

It repeats the warning once per createField() invocation in each class file.

Steps to reproduce the problem

Code generation configuration:

<plugin>
	<groupId>org.jooq</groupId>
	<artifactId>jooq-codegen-maven</artifactId>
	<executions>
		<execution>
			<id>jooq-codegen</id>
			<phase>generate-sources</phase>
			<goals>
				<goal>generate</goal>
			</goals>
			<configuration>
				<logging>WARN</logging>
				<jdbc>
					<url>${DATABASE_URL}</url>
					<user>${DATABASE_USERNAME}</user>
					<password>${DATABASE_PASSWORD}</password>
				</jdbc>
				<generator>
					<strategy>
						<name>app.licensed.jooq.CustomNaming</name>
					</strategy>
					<database>
						<name>org.jooq.meta.postgres.PostgresDatabase</name>
						<includes>.*</includes>
						<!-- Exclude schemas: https://stackoverflow.com/a/37117056/14731 -->
						<excludes>(?i:(pg_catalog|information_schema)\..*)</excludes>
						<forcedTypes>
							<forcedType>
								<name>INSTANT</name>
								<includeTypes>(?i:TIMESTAMP\ WITH\ TIME\ ZONE)</includeTypes>
							</forcedType>
						</forcedTypes>
					</database>
					<target>
						<packageName>app.licensed.database.schema</packageName>
						<directory>${project.build.directory}/generated-sources/java</directory>
					</target>
					<generate>
						<records>false</records>
						<pojos>false</pojos>
						<interfaces>false</interfaces>
						<daos>false</daos>
						<sequences>false</sequences>
						<sequenceFlags>false</sequenceFlags>
						<queues>false</queues>
					</generate>
				</generator>
			</configuration>
		</execution>
	</executions>
</plugin>

jOOQ Version

3.18.6

Database product and version

Postgresql 15.4

Java Version

openjdk version "20.0.1" 2023-04-18

OS Version

Microsoft Windows [Version 10.0.19045.3448]

JDBC driver name and version (include name if unofficial driver)

org.postgresql:postgresql:42.6.0

[cowwoc]

If you can't fix the underlying problem, antlr/antlr4#4341 shows how to suppress the warnings on behalf of end-users.

[lukaseder]

Thanks for your report.

Our company uses the -Werror javac option so warnings break our build.

I'd love to understand the rationale behind using this flag. The flag is so flawed, without being able to fine-tune every warning in the language, it is near useless. Why not use a third party tool that has the ability to fail builds more reliably, with more customisable behaviour?

A similar issue is: #14865. The user's build fails for the same reason, and the source of the problem is a bug in the compiler (in my opinion): https://bugs.openjdk.org/browse/JDK-8305250

It is unlikely that Oracle will prioritise fixing this bug, given that it's probably rare and esoteric, and given that it's "just" a warning (ignorable), and not an error (show-stopper)

If you can't fix the underlying problem, antlr/antlr4#4341 shows how to suppress the warnings on behalf of end-users.

That issue describes something we're already doing. jOOQ classes start with:

@SuppressWarnings({ "all", "unchecked", "rawtypes" })

But as the issue also says, "all" is an Eclipse specific extensions that isn't supported by javac.

Again, I fail to understand why -Werror is being used at all, given that javac doesn't allow for suppressing every individual warning.

I will obviously investigate this a bit more, but I'm not sure if anything can be done. This is a chicken-and-egg situation. The constructor should completely initialise the TableImpl, but it can't do this without calling to internal utilities (which are well aware of this problem, but it has been acceptable for 20 JDK versions, so making this a non-suppressible warning now is quite the bold move by Java 21)

[lukaseder]

One workaround you might consider is to configure javac separately for the module containing jOOQ generated code, and turn off -Werror only there. If you don't have such a dedicated module, perhaps that's a good reason to create one.

This is different in #14865, where the warning arises in client code, not jOOQ-generated code.

[lukaseder]

Anyway, I'll take the opportunity to fix a few of these warnings that may arise from generated code, including:

• Synthetic comment flag <deprecated> should produce @Deprecated annotation in addition to Javadoc #15621
• Generated "distribute members" workaround for large static initialisers produces deprecated code #15622

Having said so, some warnings are (have always been) hard to avoid, so we don't have any integration tests to make sure they do not arise. As such, -Werror on generated code will always be trouble.

In a way, this is also true for the linked antlr issue. If the code is not in your control, then I wouldn't lint it

[lukaseder]

So, I tried to reproduce this, but actually cannot. Our integration tests do not contain this warning with either JDK 20 or 21. How exactly did you configure your compiler?

[perlun]

In case this helps, I'm also seeing this now that I'm eagerly trying out JDK 21 on my machine. Here's how the compiler has been configured in my case:

options.compilerArgs = [
        '-Xlint',
        '-Xlint:-deprecation',
        '-Xlint:-path',
        '-Xlint:-processing',
        '-Werror'
]

I guess it could be the -Xlint flag that causes it in fact? When I disable it, the error seems to go away.

---

As you may guess from the above, we also tend to use -Werror in our code base. I think it's generally a good idea, but I'm possibly coming to this from a slightly different background than yours. I've used -Werror historically with gcc for C and C++ code, and there it's been considered "good practice", perhaps because warnings not being acted upon is noise. It can be extremely frustrating to try to read the gcc or javac output spewing out tons and tons of warnings - are these warnings "fine" to ignore or has it been an issue that they have not been acted upon?

-Werror (as a concept, not speaking particularly about Oracle/OpenJDK's implementation now) is a way to force people to not ignore these warnings. As a person stumbling into code written by other people, I like that. They either have to fix the warning, or silence the warning altogether. It makes my life better, at the cost of forcing people who deal with potential issues in their code or make a deliberate choice of suppressing the warning.

Now, enabling -Werror and/or -Xlint for machine-generated code... I can actually buy your point here that this is a slightly different beast. 👍

Perhaps you'd be better off by just emitting the Java bytecode right away instead of the .java classes... 😁

[perlun]

Hmm, turns out this can actually be disabled by adding this to the compilerArgs: '-Xlint:-this-escape'. 🤔 With this in place, the generated code compiles fine on JDK 21 for me. 🎉

/cc @cowwoc

[lukaseder]

Great, thanks for digging this up, @perlun.

-Werror (as a concept, not speaking particularly about Oracle/OpenJDK's implementation now) is a way to force people to not ignore these warnings. As a person stumbling into code written by other people, I like that. They either have to fix the warning, or silence the warning altogether. It makes my life better, at the cost of forcing people who deal with potential issues in their code or make a deliberate choice of suppressing the warning.

Just to be sure: I don't criticise the concept. I criticise the JDK's many non-suppressible warnings. But since this one can be suppressed, I guess the problem is "solved" for now.

I'll still keep this issue open. In the past, I've also wondered if jOOQ-generated code could avoid passing around partially initialised objects, because that's obviously not very nice (even without linting). However, I couldn't find a solution yet, which would still allow for generated code to be initialised by the static initialiser of any class.

[lukaseder]

Perhaps you'd be better off by just emitting the Java bytecode right away instead of the .java classes... 😁

The folks who have requested to make the indentation (2/4 spaces, tabs, etc.) configurable in the code generator will certainly like this idea 😅

[michael-simons]

@SuppressWarnings("this-escape")

works for fields / classes, might be an option for you @lukaseder to amend the corresponding declaration in jOOQ classes, too.

FWIW javac --help-lint is your friend for getting the possible keys.

[michael-simons]

Perhaps you'd be better off by just emitting the Java bytecode right away instead of the .java classes... 😁

So I heart there's a new API in Java 21 for that, so I wonder what you are waiting for, really… ;)

[lukaseder]

Thanks for chiming in, @michael-simons. Interesting:

this-escape          Warn when a constructor invokes a method that could be overriden in an external subclass.
                     Such a method would execute before the subclass constructor completes its initialization.

But the method is Internal::createField, which is static, so it cannot be overridden. I think the underlying lint problem still persists, but the --help-lint output doesn't share the complete story here, it seems.

[lukaseder]

Anyway, so I'll look into this soon:

1. Try to reproduce it with the lint flags
2. Try to avoid the problem entirely, e.g. by re-designing some internal API, if possible
3. If not possible, try the suppress warnings this-escape on generated code (if run on JDK 21)
4. Create a regression test with -Werror and linting turned on. I guess we can make some guarantees, even if I personally think it's not really worth it.

[perlun]

Perhaps you'd be better off by just emitting the Java bytecode right away instead of the .java classes... 😁

The folks who have requested to make the indentation (2/4 spaces, tabs, etc.) configurable in the code generator will certainly like this idea 😅

I guess I shouldn't give you any creative ideas, but another way to approach this could even be to wrap the javac invocation in a little console app that would be included in the jOOQ distribution... which would make sure to avoid -Werror and other fanciful flags. 😛 Then the generated .jar artifact would be the "generated code" by jOOQ. The fact that it writes out .java files under the hood would be an implementation detail... 🙂

(Speaking about compilation warnings, I found that even without -Xlint, some linting/warnings is already enabled - I guess this is the "default" warning set which OpenJDK considers should be enabled by default. -Xlint:none can be useful if you want to disable all warnings. According to https://docs.oracle.com/en/java/javase/17/docs/specs/man/javac.html, this is the same as specifying -nowarn. You should probably not do this on human-generated code without thinking carefully, but for machine-generated code it can clearly be useful.)

[lukaseder]

I guess I shouldn't give you any creative ideas

jOOQ already ships with the Reflect::compile utility. How hard can it be.

[cowwoc]

Sorry for taking so long to reply:

1. Yes, we are compiling with -Xlint. You'll have to do the same to reproduce this problem.
2. Yes, you can suppress this warning by adding @SuppressWarnings("this-escape") to the list of suppressions you already have at the top of generated classes.
3. You probably meant this as a joke, but just in case... I am strongly against the idea of generating bytecode instead of Java files. It makes the code much harder to debug.

My suggestion: Treat this like what it is: a warning. The compiler has identified a potential problem. So long as you review the code and it is safe then I would "sign off" on it by suppressing the warning.

When I ran into these warnings in my own codebase I had two options: Give up on a class being immutable or suppress the warning. I opted for the latter.

[lukaseder]

3. You probably meant this as a joke, but just in case... I am strongly against the idea of generating bytecode instead of Java files. It makes the code much harder to debug.

No one is going to generate bytecode here :)

Give up on a class being immutable or suppress the warning. I opted for the latter.

Yes, I wrongly assumed that this is yet another non-suppressible warning by javac, but in fact, it is suppressible, so we're all good.

[perlun]

No one is going to generate bytecode here :)

Now I'm really disappointed with you Lukas... 😂 😜

[lukaseder]

Fixed in versions:

• 3.19.0
• 3.18.7 (On JDK 21, generated code should apply "this-escape" warning suppression - Merge [#15619] #15647)
• 3.17.16 (On JDK 21, generated code should apply "this-escape" warning suppression - Merge [#15619] #15648)
• 3.16.22 (On JDK 21, generated code should apply "this-escape" warning suppression - Merge [#15619] #15649)

The fix just checks if the code generator is running on version 21+, independently of what jOOQ distribution is being used.

[norrisjeremy]

Is there a reason to not just always emit the @SuppressWarnings("this-escape") annotation regardless of the Java version?
Otherwise, wouldn't be possible to continue encountering issues if the jOOQ codegen was executed on an older Java version (< 21), but the generated code was then later compiled on a newer affected version (>= 21)?
Or am I misunderstanding?

[lukaseder]

Someone with even stronger linting opinions will complain about a Unsupported @SuppressWarnings("this-escape") warning, I'm sure.

[norrisjeremy]

I'm not sure I understand: i don't think javac cares if a particular suppression string is "valid" or not.
You can add @SuppressWarnings("FooBar") into code and it appears javac doesn't care, nor will it issue any sort of warning or error.
So I think it's safe to always emit this regardless of the Java version in play?

[lukaseder]

There were a few issues related to @SuppressWarnings in the past, I don't recall them all. It's not just javac, there are other linters out there. E.g. Eclipse complains about unknown suppress options (though, if I recall correctly, "all" suppresses that as well, but who knows if that will work everywhere).

So I think it's safe to always emit this regardless of the Java version in play?

Why isn't it safe to do it the way I did? Among all the extreme edge cases, the case where someone uses an old JDK to generate code, but a newer one to run things seems quite far fetched. If anyone actually runs into this, they can open another bikeshed here on github, and I will make a 5317th configuration flag. (I should not have said anything about the implementation 😅)

[norrisjeremy]

That's a real shame that there are broken IDE's that croak when it doesn't recognize a particular @SuppressWarnings string.

I'll have to work with our team to implement a workaround to get this all to work for our builds, since it appears we won't be able to depend on jOOQ codegen to just alway emit the suppression flag (since this will impact us, hence the reason I brought it up in the first place).

[lukaseder]

1. Why have different JDKs for these things?
2. Why not exclude jOOQ generated code from linting?

[norrisjeremy]

We'll begrudgingly figure out a way to get this to work for us locally since it appears broken IDE's need to be appeased.

[lukaseder]

It's a tradeoff. Someone's begrudging vs someone else's, and since these are bikesheds, the colour space of potential bikeshed begrudging is endless.

I won't just generate an unsupported suppress warnings flag for everyone, just to make things work for a single user (nor could I backport that option easily, because jOOQ 3.18 didn't officially support JDK 21 yet). Today, this would appease your build (which is exotic, with different JDKs being in use for different purposes for reasons not yet stated). Tomorrow, such a hack would get in your way.

While people are even more opinionated about IDEs than they are about linters and compiler warnings (I still don't understand why jOOQ-generated code is being linted in the first place), the Eclipse compiler is a very popular one also outside of the IDE. Visual Studio Code is using it, for example. This is not an IDE feature but a vendor specific compiler feature.

But there. I have created a feature request where users can specify the output JDK version:

• Add a code generation configuration to specify the output JDK #15654

In the future, such a flag might allow for overriding the behaviour of jOOQ's code generator when it has a DETECT_FROM_JDK flag. Currently, there are 3 such cases (this one here and the 2 others I mentioned in that issue). I don't think it's worth making this configurable yet, but who knows.

Note, as a final workaround: It's also very easily possible to override the JavaGenerator::printClassAnnotations method for all classes and suppress all these warnings. Starting from jOOQ 3.19, you can even create an inline extension of the JavaGenerator that overrides stuff, compiled in-memory of the generation process:

• Support ad-hoc compilation of programmatic Generator or Database objects in the code generator #15213

I hope any of the above appeases the begrudgement

[cowwoc]

As to why JOOQ code is being linted... It's a mess to use different compilation flags for generated code than the main codebase: https://stackoverflow.com/a/39995250/14731

Also, the extra-paranoid among us want to receive notification when one of our dependencies introduces potential bugs and/or security vulnerabilities into the codebase.

[lukaseder]

As to why JOOQ code is being linted... It's a mess to use different compilation flags for generated code than the main codebase: https://stackoverflow.com/a/39995250/14731

That looks simple to me... I was assuming you'd move jOOQ generated code to a separate module, but the option with two executions is also nice. You can also generate jOOQ code outside of src/main/java, which seems cleaner than including/excluding packages

Also, the extra-paranoid among us want to receive notification when one of our dependencies introduces potential bugs and/or security vulnerabilities into the codebase.

Nothing wrong with that. jOOQ has a few builds running just for that (separate builds though, as they don't require immediate action). I'd recommend a separate build for that purpose as well, just because, well, this kind of stuff quickly turns into busywork.

Anyway, I think the issue is fixed. If there is enough reason to add more configuration, I can do that. Until then, there are tons of workarounds, including e.g. -Xlint:-this-escape for the time being, until the code generation JDK can be upgraded.