Maybe, instead of relying on static state (and the initialization/reset that requires) we could pass around a special error logger in the AstAnalysisConfiguration I introduce in #1426. This is what the object's made for, and it would be in scope at the current call sites of your ThreadContextHolder.

I agree with @jsotuyod that the error reporting should be user-friendly, though I guess that can be handled by the report renderers? Maybe using a subclass of ProcessingError would allow us to filter them out at that point? In the less verbose report formats, we could also only display the package names when there are very many missing classes.

I've come back to this after some time…

I don't think passing around AstAnalysisConfiguration will cut it here for a number of reasons:

1. At some point we may want type res to be lazy, as it will get far more CPU-intensive as it progresses (ie: resolving return types of method calls in the presence of overloads / shadows / overrides). This is something we have already wrote about at some point.
2. The AstAnalysisConfiguration refers to a single file, and not generally to the current analysis.
3. We know from experience, some missing classes may not be found during preprocessing, but during rule analysis, hence we have code as this in rules such as MissingOverrideRule:

Moving the AstAnalysisConfiguration around through all analysis / having rules able to access it is going to bloat our interfaces. Moreover, if at some point we realize we need it somewhere else, it's going to force us into breaking changes to keep adding it as a parameter all along the call stack.

I think we should aim for the following:

Have a new AnalysisContext with info about a given PMD run. This should be easily globaly accessible from anywhere, but we probably can't just make it static as we need to be careful about multiple analysis under the same JVM, as Gradle can do. Maybe, the best approach would be:

1. Have a new AnalysisContextHolder, which uses a ThreadLocal to retrieve it.
2. Have PmdThreadFactory make sure the AnalysisContextHolder is populated with the one and only AnalysisContext for the current run.

It could provide a simple method such as noteIncompleteAuxclasspath(LinkageError e) and maybe other overloads; and have a single place to extract the missing class FQCN, and store it in a Set.

After analysis, we can dump these into the Report before rendering, thus saving us from sprinkling all the codebase with accesses to Report.

Notice, another benefit of this approach is that introduces no breaking API changes, so we can do it on master at any time.

@jsotuyod I don't really like that approach.

2. The AstAnalysisConfiguration refers to a single file, and not generally to the current analysis.

TBH I think we should add a getLanguageVersion() to the Node interface and stop carrying around in ThreadLocals in RuleContext or so. A node isn't going to change its language, and if a node doesn't know its own language, then we can't support embedded languages. So we can remove that from the AstAnalysisContext anyway. (Same for the RuleContext#getCurrentFile. It should be accessible on the RootNode).

If we do that then the analysis context is not specific to a file anymore, and it's basically your AnalysisContext

3. We know from experience, some missing classes may not be found during preprocessing, but during rule analysis, hence we have code as this in rules such as MissingOverrideRule

I don't think we should base any work on this hypothesis. If we have a richer data model for symbol/type resolution, that rule doesn't need to process Class objects. Same thing for eg UnnecessaryFullyQualifiedName and such. I'd rather shift the responsibility of processing Class objects away from rules, because that will simplify them, and allow sharing insight between all rules. I've already written about that extensively in #1566 and in our meeting briefs so won't go back into that.

1. At some point we may want type res to be lazy,

This is irrelevant. Making it lazy doesn't mean we can't use a context object. We could have a lazy type resolver object per compilation unit, created with a context object.

noteIncompleteAuxclasspath(LinkageError e)

I think passing around a special logger object would be more general and scale better. Eg with methods like recoverableException(Throwable t); warn(String message);, etc, which would be handled in a generic way (maybe merging stack traces of the same exceptions, which btw would also be useful for exceptions occuring in rules). Using java.util.logging.Logger is easy but everything is static, so once again not analysis-scoped.

In general I think "more threadlocals" shouldn't ever be a long term design, precisely because we don't know exactly how PMD is executed outside of our codebase. It makes data ownership obscure, makes the app harder to understand. Reifying global things into context objects and passing them around is a much better place to start from I think. There are not many places where the parameter passing is needed, and nearly all of them are in the internals of PMD, which is internal API.

Closing this without merging it. It is irrelevant, will be replaced by a solution for #3914.