jspecify / jspecify

An artifact of fully-specified annotations to power static-analysis checks, beginning with nullness analysis.
http://jspecify.org
Apache License 2.0
524 stars 29 forks source link

Separate directories of sample inputs for "not enough information" and "mismatch," maybe for other kinds of divisions? #130

Open cpovirk opened 3 years ago

cpovirk commented 3 years ago

Early last month, we had discussed splitting the samples into "strict" and "lenient" directories. We agreed to give it a shot and see how it goes.

I gave it a shot, and I came away with 3 concerns:

  1. First, it prevents us from grouping together similar sample inputs. For example, look at Overrides.java. It has a base interface with 3 methods -- one for returning each of non-null, unspecified, and nullable values. Then, it has 3 subinterfaces that override each of those 3 methods with each of the 3 nullness values. It's nice that we can show all 9 cases together in one file. If we were to split the file, we would have 6 cases in one file and 3 in the other. That's doable, but it makes it harder to see what interpretation we suggest in all cases -- or even to see that we have enough samples to cover all cases. We'd likely end up either with comments pointing from one file to another or with a completely parallel set of files under each directory.

    • [edit: And in which file do the "passing" (non-"error," non-"warning") samples belong?]
  2. As someone (mernst?) had suggested, we are seeing requests for additional kinds of divisions:

    This suggests a couple things:

    • If we are going to use separate directories, we should think carefully about which kinds of divisions justify them, or else we will end up with hierarchies that go 4 deep.
    • We are likely to use different "tag comments" for some divisions. Consequently, checkers are likely to have multiple reasons to distinguish among such comments. At that point, one additional tag for "not enough information" vs. "mismatch" is a fairly minor difference.
  3. I think that even lenient checkers will be interested in the "not enough information" tests. Specifically, they'll want to make sure that they do not report errors in those cases. Handling the "not enough information" comments should be easy: Just pretend that they don't exist, and your tests will automatically check that there are no errors there.

For these reasons, I would like to continue to let a given file contain both "not enough information" and "mismatch" cases.

We can always revisit. As we discussed just today, it makes sense to focus our attention on the issues that come up when people work on their tool implementations. So, if the current setup causes problems for those people, please report it here.

mernst commented 3 years ago

For these reasons, I would like to continue to let a given file contain both "not enough information" and "mismatch" cases.

I concur.