Unused result warnings

1

Greetings all,

I’d like to propose a feature where the compiler would warn users if they ignore the result of specially annotated functions. Here’s some examples of bugs:

val s = "Hello"
s.substring(2) // Ignoring the result is always a bug or pointless

data class Person(val firstName: String, val lastName: String)

val p = Person("Andrew", "O'Malley")
p.copy(firstName = "Bill") // Bug




val set = Sets.of("Hello") // Persistent set (e.g. from Dexx)
set.add("there") // Bug

I see the number of cases of such bugs growing as immutable structures and functional approaches become mainstream.

I propose allowing functions to declare that their results must be used with a mustUse annotation. If they are not used the compiler will emit a warning.

e.g.

mustUse fun String.substring(beginIndex: Int) : String = ...

Precedents:

  1. IntelliJ already has this feature in the Java plugin for String.substring(). Presumably the rule is hard-wired somewhere however
  2. I believe Rust has a “unused_must_use” lint mode coupled with a “#[must_use]” directive. It only seems to apply at a trait level, not a function level however.


Any thoughts? If the idea has any traction I can raise an issue for it.

Cheers,
Andrew

1 Like
2

Looks more like an IDE feature to me

3

Fyi, Nimrod's solution is to explicitly require a discard statement for these cases, e.g. 

val s = "Hello"
discard s.substring(2)

data class Person(val firstName: String, val lastName: String)

val p = Person(“Andrew”, “O’Malley”)

discard p.copy(firstName = “Bill”)


val set = Sets.of(“Hello”) // Persistent set (e.g. from Dexx)

discard set.add(“there”)

Omitting “discard” in the above cases is a static error. I really like this solution as the “discard” makes it obvious that one executes a function only for its side effects. Of course, no one should write code like the above, i.e. the lines starting with “discard” should not even be there.

4

Interesting. It looks like Nimrod always warns and discard is an opt-out.

I don’t think that would be feasible in Kotlin as there’s a multitiude of things in the Java space that return results that no-one (generally) is interested in. e.g. the Collections.add and Map.put return values that are rarely used.

Cheers,
Andrew

5

Hi Andrey,

I suppose it currently is for IntelliJ’s java plugin, but:

  • How would libaries add new rules to the IDE? Or is the IDE limited to understanding the standard library with a fixed set of rules?
  • How would rules be shared between IntelliJ and the Eclipse plugin in progress?


Anyway, I respect your judgement for not wanting to add bloat to the compiler. :slight_smile:

I guess such things call be handled by a static analysis tool like http://findbugs.sourceforge.net/ in a post compilation step for people that want it.

Cheers,
Andrew

6

I've found that FindBugs and JSR 305 supports my use case using via the @CheckReturnValue.

So I could “import CheckReturnValue as mustUse” and run FindBugs to get what I want.

Here’s an example: http://www.sw-engineering-candies.com/blog-1/findbugstmwarningsbysamplenonnullandcheckreturnvalueofjsr-305

Not perfect, but certainly workable.

Cheers,
Andrew

7

IDE can understand annotations too.

8

Ah, that makes sense - you're not saying the idea doesn't have any merit, just that it doesn't belong in the compiler itself.

Thanks for the clarifcation - I’ll let you get back to more important tasks. :slight_smile: