[core] Language properties

[oowekyala]

I think the most generic solution to the problems we have when it comes to configuring languages, is to configure Language instances directly via properties.

Example use cases:

• We currently have a CLI switch for the suppress marker, another for the auxclasspath.
  • The auxclasspath is only used by JVM languages (currently only Java). We try to reuse our special PmdAsmClassLoader, bound to the classloader created for the auxclasspath. That classloader is stored in a static variable. It's overwritten if you launch an analysis with another auxclasspath. This means, analysing different projects concurrently will overwrite each other's classloader repeatedly, undermining reuse. This also means the only way to reclaim the memory for this classloader is to launch another analysis, otherwise it will stay in the static variable. Clearly there's a problem with lifecycle here. See here
  • The suppress marker is set for every language globally. It's the only use case for ParserOptions, which otherwise could be removed. It would make sense to configure this language by language
• Filename extensions are not configurable, though it has been a feature request for a long time: [xml] Please externalize the mapping of file extensions to languages as preference file  #461, [pom] Maven POM Rules don't check files named "pom.xml" #1540, [xml] Allow to check Salesforce XML Metadata using XPath rules #2133
• Language-specific tabsizes [core] Support configurable tabsize for AST node position in reports #292
• Apex-specific configuration [apex] Apexlink POC #2830
• Visualforce-specific configuration [vf] Provide expression type information to Visualforce rules to avoid false positives #2864

The core of this proposal is to make LanguageRegistry non-static, and giving it a proper lifecycle. This makes it so, that analysis-global state (like the classloader) can be stored in the language instance, and shared with eg the parser and other processing stages, without having to jump through hoops to figure out if we have the same parameters as before.

Consider making the service that is loaded by ServiceLoader not Language directly, but a LanguageLoader. This object creates a Language instance with property descriptors. The initialization takes care of configuring the language with all those things mentioned above.

For example, a LanguageLoader interface could look like so:

interface LanguageLoader<P extends PropertySource> {

    String id();

    P newPropertyBundle();

    Language createLanguage(P properties);

}

• newPropertyBundle creates a PropertySource and defines the PropertyDescriptors accepted by the language
• createLanguage creates a language instance given a configuration

Eg the java implementation of this would be

class JavaLanguageLoader implements LanguageLoader<JavaLanguageProperties> {


    @Override
    public String id() {
        return JavaLanguageModule.TERSE_NAME;
    }

    @Override
    public JavaLanguageProperties newPropertyBundle() {
        return new JavaLanguageProperties(); // defines some properties (auxclasspath, suppressMarker, filePatterns)
    }

    @Override
    public Language createLanguage(JavaLanguageProperties properties) {
        // in this language instance we can initialize the special classloader before analysing the first file
        return new JavaLanguageModule(
            properties.getFilePatterns(), // more general than file extensions, eg supports "pom.xml"
            properties.getSuppressMarker(),
            properties.getAuxclasspathClassloader()
        );
    }
}

Language properties can be set with this simple CLI extension: -L<langId>:<propName> <value>.
For example -Lxml:fileNamePatterns 'pom.xml,*.fxml', or -Ljava:auxclasspath 'some;cp'.

The initialization process of a language registry would look like so:

1. LanguageRegistry uses ServiceLoader to load a bunch of LanguageLoader instances
2. -L options of the command line args are partitioned by language ID
3. Foreach language loader, create a language instance like so:
   1. create a property bundle (newPropertyBundle)
   2. call setProperty for each -L switch targeting the given language (this will throw errors for misconfigurations)
   3. call createLanguage with that property bundle
4. Create a LanguageRegistry wrapping this set of languages

Language and LanguageRegistry can implement AutoCloseable. This allows the language registry to be used in a try statement.
So, very high in the PMD call stack, we can have something like

    public static void main(String[] args) {
           
        // langProperties := properties from the CLI, partitioned by language 
        // serviceClassLoader := classloader used by ServiceLoader
        // LanguageRegistry::loadWithProperties is the loading routine described above

        try (LanguageRegistry registry = LanguageRegistry.loadWithProperties(serviceClassLoader, langProperties)) {
            // load rulesets, execute, etc, using this registry
            // Other analyses use different LanguageRegistries, so are independent from this one
        }
        // classloader & other resources allocated on construction are reclaimed
    }

I think this mechanism solves long-standing issues with our configuration model, namely

• adding options to a language implementation requires changing everything, from the upper layers like Configuration and the CLI itself (or the schema, similar to [core] Abstract away optional AST traversals (first step) #1426), to the implementation of the feature, that is spread ad-hoc over all the pmd-core codebase. Most languages probably don't need all those options anyway (eg the auxclasspath).
• there's no "analysis scoped" objects, which means we have to use dirty tricks like threadlocals or
  the horror that's PmdAsmClassLoader to share objects through an analysis. Such "tricks" make PMD harder to use in a concurrent setting. There's no cleanup of static state at all.
• ParserOptions is a pain to use and could be removed (see wiki)

It gives us a simple and general extension mechanism to configure language-specific behavior,
without affecting other languages, and without needing changes to pmd-core. For example, we could use that to enable logging of detailed type-resolution-specific debug information (missing classes, type inference failures), which would fit well into the updated java module

[adangel]

Thanks for writing this up! These are very good ideas.

I've right now only one comment regarding file extensions/patterns: I think, the reason, why PMD actually looks at the file extension, is this: it was envisioned to have a ruleset that references rules of different languages and you would call PMD on a project directory which contains files of these different languages. Now PMD has to figure out, which rules should be applied to which files - since trying to parse a file with the wrong parser will result in many errors, which are not real errors.
When calling PMD from CLI, our CLI impl will find the files and feed it into analysis. When calling PMD from ant, a ant fileset is used. the maven-pmd-plugin searches the file on its own.
The only benefit I see of this is, that you need to run PMD only once and you could get a single report for all the different languages in the project versus calling PMD multiple times with rulesets tailored for each language and getting multiple reports.
This language property would definitively be a solution for this, I just wanted to make sure, we actually need it...
If we provide sensible defaults for everything, I could even imagine that one could just run PMD CLI without any arguments and we'll run analysis of all files we understand and provide a nice (html) report - that would be an easy way to start getting to know PMD... and that would require, that we are able to figure out which rule should be applied to which file.

[linusjf]

What's the assumption made about the rules file? Is PMD expecting separate rule config files for each language? This could be the norm but it's quite likely that a project would support multiple languages such as java, JSP, xml in the same project.

Wouldn't it be better to have these language properties reside in a properties file unique to that project and have the ruleset file refer to these properties somewhat like Ant does? This lends itself to some extensibility and the language properties can be prefixed by the language identifier. This only becomes complex if you are supporting multiple versions of the same language i.e., jdk ; some naming convention can be arrived at to resolve this.

[oowekyala]

My point is simply that language properties ought to reside with the language rules.

I don't think this necessarily holds. More importantly this issue is mostly about the lifecycle, and the cli extension is just the easiest way to close the loop. I suggest we start by implementing that, and then discuss better ergonomics if need be -> but somewhere else.

[adangel]

Done for PMD 7 via #4060