tulipcc/ParserGeneratorCC

Remove encoding-less parser constructors

sfuhrm opened this issue ยท 8 comments

ph-javacc-maven-plugin warns in the docs about the constructors without an encoding.

The parser constructors without a character encoding throw a NullPointerException.

It doesn't make sense to keep these calls since they are breaking the API because the encoding is now mandatory. Before that, it seems to have been assigned by default if no encoding was given.

I'd suggest to remove the parser constructor calls without character encoding.

This is the exception that is thrown when calling the 'buggy constructors':

Caused by: java.lang.NullPointerException: charsetName
        at java.base/java.io.InputStreamReader.<init>(InputStreamReader.java:99)
        at de.einsundeins.netcfg.preprocessor.templatepreprocessor.SimpleCharStream.<init>(SimpleCharStream.java:69)
        at de.einsundeins.netcfg.preprocessor.templatepreprocessor.SimpleCharStream.<init>(SimpleCharStream.java:78)
        at de.einsundeins.netcfg.preprocessor.templatepreprocessor.TemplatePreprocessor.<init>(TemplatePreprocessor.java:263)
        at de.einsundeins.netcfg.preprocessor.templatepreprocessor.TemplatePreprocessor.<init>(TemplatePreprocessor.java:253)
        .....
phax commented

Well, alternatively I could fix the NullPointerException ;-)

Thanks for quick response ๐Ÿ‘

InputStreams deliver bytes, not chars. But a parser needs chars. So I think it makes sense to either go with a Reader or an InputStream+encoding.

Since the problematic calls are using an InputStream as the source, it would be better to drop the variants without encoding all over. I'd go one step further and make all InputStream constructors only a mapping to some meaningful Reader instance.

I guess the old JavaCC code guessed (!?) what the encoding is. Guessing is bad.

I had the NPE-behaviour in old code that used JavaCC before and was a bit surprised that I get runtime errors after migrating to ph-javacc-maven-plugin (thanks for that!).

My opinion: Compile time errors are much better if you need to touch your code anyway.

phax commented

Hi,
so I scanned through the sources and could not find any template that contains a constructor without a charset.
What version of the ph-javacc-maven-plugin / parser-generator-cc are you using?
The latest versions are 4.1.4 (ph-javacc-maven-plugin) and 1.1.3 (parser-generator-cc)
Sorry for looking dumb ;-)

Never mind, we're here to master complexity ;)

The plugin used is 4.1.4:

      <plugin>
        <groupId>com.helger.maven</groupId>
        <artifactId>ph-javacc-maven-plugin</artifactId>
        <version>4.1.4</version>
        <executions>
          <execution>
            <phase>generate-sources</phase>
            <id>javacc</id>
            <goals>
              <goal>javacc</goal>
            </goals>
          </execution>
        </executions>
      </plugin>

and this uses

    <dependency>
      <groupId>com.helger</groupId>
      <artifactId>parser-generator-cc</artifactId>
      <version>1.1.3</version>
    </dependency>

I think you are looking for this and this?

Possibly there are more funny things expecting NULL friendly constructors.

phax commented

Ahhhh :D - thank you ;-) Too many configuration options. ๐Ÿ˜Š

phax commented

Since I always use "modern" mode I never stumbled upon that one.... sigh

phax commented

Thanks for bringing this to my attention - it will be fixed in the next release.
From my point of view this change alone is too little for a new release ;-)