NullAway icon indicating copy to clipboard operation
NullAway copied to clipboard

System.exit()

Open fprochazka opened this issue 6 years ago • 1 comments

I've found a case, that is not handled properly (I think). I expect the JVM to shutdown when I call System.exit(1); and therefore I'd expect this statement to behave as a control-flow guard, similar to putting there a return;. But it is ignored.

@Nullable CommandsGroup commandBean = getCommandBean(commandNameParts[0], context);
if (commandBean == null) {
    System.err.println("Command " + commandName + " not found.");
    System.exit(1);
}

commandBean.runCommand(commandNameParts[1], context.getBean(ApplicationArguments.class));

produces

[ERROR] ConsoleApplication.java:[54,24] [NullAway] dereferenced expression commandBean is @Nullable
    (see http://t.uber.com/nullaway )

The fix in my code is trivial

@Nullable CommandsGroup commandBean = getCommandBean(commandNameParts[0], context);
if (commandBean == null) {
    System.err.println("Command " + commandName + " not found.");
    System.exit(1);

} else {
    commandBean.runCommand(commandNameParts[1], context.getBean(ApplicationArguments.class));
}

Alternatively this

@Nullable CommandsGroup commandBean = getCommandBean(commandNameParts[0], context);
if (commandBean == null) {
    System.err.println("Command " + commandName + " not found.");
    System.exit(1);
    return;
}

commandBean.runCommand(commandNameParts[1], context.getBean(ApplicationArguments.class));

But IMHO this is something the NullAway should handle.

I wasn't looking very hard, but I don't believe there is a way to escape the System.exit(1); (apart from the shutdown handler hook, that is executed in different thread and there doesn't affect the method and thread calling the exit).

maven: effective pom
    <plugin>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.8.0</version>
        <dependencies>
          <dependency>
            <groupId>org.codehaus.plexus</groupId>
            <artifactId>plexus-compiler-javac-errorprone</artifactId>
            <version>2.8.5</version>
            <scope>compile</scope>
          </dependency>
          <dependency>
            <groupId>com.google.errorprone</groupId>
            <artifactId>error_prone_core</artifactId>
            <version>2.3.3</version>
            <scope>compile</scope>
          </dependency>
        </dependencies>
        <configuration>
          <compilerId>javac-with-errorprone</compilerId>
          <forceJavacCompilerUse>true</forceJavacCompilerUse>
          <source>1.8</source>
          <target>1.8</target>
          <compilerArgs>
            <arg>-Werror</arg>
            <arg>-Xlint:all,-fallthrough,-processing,-serial,-classfile</arg>
            <arg>-parameters</arg>
            <arg>-XDcompilePolicy=simple</arg>
            <arg>-Xep:NullAway:ERROR</arg>
            <arg>-XepOpt:NullAway:AnnotatedPackages=com.cogvio.dl</arg>
            <arg>-XepOpt:NullAway:TreatGeneratedAsUnannotated=true</arg>
            <arg>-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true</arg>
          </compilerArgs>
          <annotationProcessorPaths>
            <path>
              <groupId>org.springframework.boot</groupId>
              <artifactId>spring-boot-configuration-processor</artifactId>
              <version>2.2.0.M1</version>
            </path>
            <path>
              <groupId>org.springframework</groupId>
              <artifactId>spring-context-indexer</artifactId>
              <version>5.2.0.M1</version>
            </path>
            <path>
              <groupId>com.uber.nullaway</groupId>
              <artifactId>nullaway</artifactId>
              <version>0.7.2</version>
            </path>
          </annotationProcessorPaths>
          <showWarnings>true</showWarnings>
          <parameters>true</parameters>
        </configuration>
      </plugin>

fprochazka avatar May 16 '19 14:05 fprochazka

You're right that NullAway doesn't currently model System.exit() correctly. I think using it outside of a main driver class is regarded as an anti-pattern since, as you point out, there is no easy way for calling code to stop or catch it. We don't have a straightforward way to model this right now, but if we get time we can look into it. I wouldn't consider it particularly high priority given the rarity of use. Hopefully your workarounds are sufficient for now.

msridhar avatar May 16 '19 15:05 msridhar