Support cross-package nolint parsing

[yuxincs]

We can report errors on pkg A when we are analyzing pkg B due to our multi-pkg inference. However, nogo’s support for nolint comments is based on reading the AST from current package.

That is, nogo does not know we have a nolint comment in pkg A when analyzing pkg B , but NilAway can report an error there, and hence cannot be suppressed.

The other linter implementations most likely follow the same logic.

We’ll need to implement own cross-pkg nolint support (i.e., reading the nolint and writing it to facts, then read it in downstream analysis to decide whether to report a certain error or not).

[davecb]

I think you're constrained to use the "//nolint:" notation, but just to clarify my understanding, am I correct in thinking what you mean it to express is "this cannot be nil by construction"?

I just ran nilaway on a program that of mine, and added // postcondition: x is non-nil to remind me that the library code I just called produces a non-nil result. (I sort of miss #define ASSERT(...) from a less-modern language (;-))

[yuxincs]

@davecb Not exactly :)

//nolint: here basically just suppresses errors reported, without meaning anything else.

You're probably thinking about adding annotations to parameters & returns, similar to what Java has (@Nullable and @Nonnull).

In NilAway we try to avoid adding annotation burden onto devs since it's not a builtin language feature unlike Java. But we do parse comments like

//nonnil(result 0)
func foo() (*int) {....}

Where the return value would be assumed nonnil in NilAway's perspective. But those are mostly for testing purposes (do not rely on this feature in your code, though).

In the future, we plan to add a tiny "annotation"-like library that NilAway understands, and most importantly will log and/or panic if the assumption is broken.

func main() {
  // say for some complex code in foo NilAway just cannot reason the return value is nonnil, but you as a developer know that for sure.
  v := foo() 
  v = castToNonnil(v) 
  print(*v) // NilAway won't complain here anymore.
}

where castToNonnil does nothing but return the same value, in the meantime it can log and/or panic if the input is ever nil (i.e., breaks the assumption) to (1) combat false positives in NilAway in edge cases, and (2) still raise awareness of code defects.