Additional units for unit checker; fixes #4116

[rkraneis]

No description provided.

[rkraneis]

I only implemented @Volume for now in the obvious way (including updates to tests and documentation); other interesting units are mentioned in issue #4116. When it is clear that I didn't miss anything technical for @Volume and which additional units to support, I will add those.

[mernst]

This looks great. Thanks very much for the contribution. I only fixed one typo.

Please go ahead and add other units, and let me know when I should re-review.

[mernst]

@rkraneis Please let us know if there is anything we can do to help.

I'm not trying to nag -- it's just that we are eager for this improvement. Thanks!

[rkraneis]

@mernst I'm a bit tight on time at the moment, but will be able to tackle the topic again over Easter.

[mernst]

OK, thanks for letting me know. Good luck with everything.

[rkraneis]

Hi @mernst, two things actually do give me a little bit of a hard time (both around testing):

1. IntelliJ always claims the test files (checker-framework/checker/tests/units/*.java) to be error free. Is this related to the non-trivial Gradle setup? (I'm neither an IntelliJ nor a Gradle user, but my IDE of choice completely choked on the build scripts, so this misbehaviour may totally be my fault). Actual compilation issues can only be (kind of?) seen when manually running ./gradlew :checker:UnitsTest; which brings me to my second point
2. Even when running with the advertised additional debug parameter ./gradlew -Pemit.test.debug :checker:UnitsTest I don't get that helpful output:

$ ./gradlew -Pemit.test.debug :checker:UnitsTest

> Task :checker:UnitsTest

org.checkerframework.checker.test.junit.UnitsTest > run[/home/rene/Projekte/typetools/checker-framework/checker/units] STANDARD_OUT
    Running test using the following invocation:
    javac -processor org.checkerframework.checker.units.UnitsChecker ...
    ---------------- start of javac ouput ----------------

    ---------------- end of javac ouput ----------------

org.checkerframework.checker.test.junit.UnitsTest > run[/home/rene/Projekte/typetools/checker-framework/checker/units] FAILED
    java.lang.AssertionError: 104 out of 138 expected diagnostics were found.
    6 unexpected diagnostics were found:
    SubtractionUnits.java:167: error: cannot find symbol
    SubtractionUnits.java:168: error: cannot find symbol
    SubtractionUnits.java:35: error: (type.checking.not.run)
    TypeVarsArrays.java:1: error: (type.checking.not.run)
    Units.java:7: error: (type.checking.not.run)
    UnqualTest.java:3: error: (type.checking.not.run)
    34 expected diagnostics were not found:
    SubtractionUnits.java:200: error: (assignment.type.incompatible)
    SubtractionUnits.java:204: error: (assignment.type.incompatible)
    SubtractionUnits.java:208: error: (assignment.type.incompatible)
    SubtractionUnits.java:212: error: (assignment.type.incompatible)
    SubtractionUnits.java:216: error: (assignment.type.incompatible)
    SubtractionUnits.java:220: error: (assignment.type.incompatible)
    SubtractionUnits.java:224: error: (assignment.type.incompatible)
    SubtractionUnits.java:228: error: (assignment.type.incompatible)
    SubtractionUnits.java:232: error: (assignment.type.incompatible)
...

(that is, there is no “javac output”, so figuring out what I missed is a bit tiresome)

And, as stated previously, I'm doing actual work on rkraneis:additional_units_private to not unnecessarily trigger azure builds on typetools infra but on mine.

[rkraneis]

Another problem around automatic reformatting:

    // :: error: (assignment.type.incompatible)
    @N(Prefix.giga) int notN = N;

always gets reformatted to

    // :: error: (assignment.type.incompatible)
    @N(Prefix.giga)
    int notN = N;

I can rewrite this to

    @N(Prefix.giga)
    // :: error: (assignment.type.incompatible)
    int notN = N;

to make the tests still pass, but it's definitely not optimal. This does not happen for @m(...), though.

[rkraneis]

And one more observation: Unfortunately, the unit C (instead of the unrepresentable °C) is already used in the units checker for the derived SI unit degrees Centigrade, so something else needs to be thought of for the electric charge Coulomb, if it should be added.

[mernst]

@rkraneis I apologize for my delay in responding; I didn't notice the GitHub notifications.

Thanks again for these changes and for the comments about what was difficult for you.

Regarding tests:

• I'm not sure why the "start of javac output" text is empty. Thanks for pointing that out.
• When I see an error in the abbreviated "unexpected diagnostics were found", "expected diagnostics were not found" format, if I cannot immediately figure out what went wrong, I typically run the checker on just the given file to see the full output.

Regarding formatting:

• The formatting is controlled by https://github.com/plume-lib/run-google-java-format .
  This comment says how to update its list of type qualifiers. I can do that.

I don't have a concrete naming suggestion about Colomb vs Centigrade. I'm happy to use whatever you suggest.

[mernst]

Thanks very much for these additional units! I appreciate your contribution.