[GROOVY-8547] Factory pattern side-stepped by CompilerConfiguration/ParserVersion

• Type: Improvement
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Fix Version/s: 3.0.0-beta-3
• Component/s: parser
• Assignee: Paul King
• Reporter: Eric Milles
• Created: 16/Apr/18 01:14
• Updated: 12/Aug/19 12:56
• Resolved: 15/Jul/19 13:29

Description

It seems to me the selection of the Antlr2 or Antlr4 parser could be implemented within a default implementation of ParserPluginFactory instead of introducing ParserVersion with separate get/set on CompilerConfiguration. I don't see the need for static factory methods antlr2() and antlr4() on ParserPluginFactory, their use from CompilerConfiguration and the strange class loading in ParserPluginFactory.

The factory pattern appears to me to be side-stepped in favor of a version constant. Is there still meaning in offering a config option of setting a ParserPluginFactory? Has this been used ever in the past?

Couldn't this suffice for a default in CompilerConfiguration? Then ParserVersion could be dropped.

    public ParserPluginFactory getPluginFactory() {
        if (pluginFactory == null) {
            pluginFactory = new ParserPluginFactory() {
                @Override
                public ParserPlugin createParserPlugin() {
                    return Boolean.getBoolean(GROOVY_ANTLR4_OPT) ? new Antlr4ParserPlugin() : new AntlrParserPlugin();
                }
            }              
        }
        return pluginFactory;
    }

Issue Links

• links to: GitHub Pull Request #967