Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[java] Report unannotated receiver parameter #2030

Open
linusjf opened this issue Sep 22, 2019 · 14 comments
Open

[java] Report unannotated receiver parameter #2030

linusjf opened this issue Sep 22, 2019 · 14 comments
Labels
a:new-rule Proposal to add a new built-in rule

Comments

@linusjf
Copy link

linusjf commented Sep 22, 2019

Affects PMD Version:
All.

Rule:
Suggested Name:
AnnotateReceiverParameter
Non-static methods and constructors should not use "this" as a parameter unless it's annotated. New rule needed.

Description:

Example of code with violation:

import org.checkerframework.checker.nullness.qual.NonNull;

public class Foo {
    public void bar(Foo this, int x) { // violation should be reported here
        System.out.println(x);
    }
    public void bar2(@NonNull Foo this, int x) { // violation should be reported here, too
        System.out.println(x);
    }
}

A valid code sample:
(from https://www.logicbig.com/tutorials/core-java-tutorial/java-8-enhancements/explicit_receiver_parameters.html)

For example this code may be a compile time restriction for the callers if they are not calling calc
method on an instance of Calculator which has presumably been loaded from the server.

public class Calculator {
    public Result calc(@ServerObject Calculator this){ // no violation here, valid use case
       this.getSomething();
        ...
    }
 }

Best described here:

Non-static methods and constructors to non-static inner classes have first parameter "this", as I mentioned in 2002 (https://t.co/EeB0BZOjp9). Since #Java 8, we can specify this explicitly. We call it a "receiver parameter" (§8.4 of JLS13) and allows us to annotate "this". pic.twitter.com/hejhaNlV3L

— Heinz Kabutz (@heinzkabutz) September 21, 2019
package javapuzzles;

// https://twitter.com/heinzkabutz/status/1175283793592233985
@SuppressWarnings({"PMD.ShortClassName"})
public class Foo {
  private final int i;

  public Foo(int i) {
    this.i = i;
  }

  public void method(Foo this) {
    System.out.println(this.i);
  }

  public void method(Foo this, String... args) {
    System.out.println(this.i);
    for (String arg: args)
      System.out.println(arg);
  }

  public static void main(String... args) {
    Foo foo = new Foo(42);
    foo.method();
    Bar bar = new Bar(42);
    bar.method();
    bar.method("hello", "good morning", "aloha");
  }
}

@SuppressWarnings({"checkstyle:onetoplevelclass", "PMD.ShortClassName"})
class Bar extends Foo {
  Bar(int i) {
    super(i);
  }

  @Override
  public void method() {
    // empty method
  }

  @Override
  public void method(String... args) {
    // empty method
  }
}

It can become confusing.

package javapuzzles;

import java.util.Iterator;
import java.util.NoSuchElementException;

public class NonAnonymousReceiver {

  abstract class EmptyIterator implements Iterator<String> {
    public boolean hasNext(EmptyIterator this) {
      return false;
    }

    public String next(EmptyIterator this) {
      throw new NoSuchElementException("No next element");
    }
  }

  public Iterator<String> emptyIterator2() {
    return new EmptyIterator() {};
  }
}
@oowekyala
Copy link
Member

Your link is dead, but I found the thread. The receiver parameter is not necessarily useless, as Brian Goetz himself argues in that thread. It's only useless if it is not annotated (or just annotated NonNull or a variant).

Feel free to submit a PR

@oowekyala oowekyala changed the title [java] Non-static initializers and methods should discourage use of "this" as parameter. [java] Report unannotated receiver parameter Sep 22, 2019
@oowekyala oowekyala added the a:new-rule Proposal to add a new built-in rule label Sep 22, 2019
@linusjf
Copy link
Author

linusjf commented Sep 22, 2019

Yes, I updated the documentation. The tweet content is now embedded.

@linusjf
Copy link
Author

linusjf commented Sep 22, 2019

Added code sample to description.

@linusjf
Copy link
Author

linusjf commented Sep 23, 2019

#719

There's also a code sample in here.

@linusjf
Copy link
Author

linusjf commented Sep 23, 2019

https://bugs.openjdk.java.net/browse/JDK-8195646

Modified code in bug to have another code sample that compiles. That also illustrates why an anonymous class' this must refer to its parent instead since there's no way of referring the anonymous class itself.

@linusjf
Copy link
Author

linusjf commented Sep 23, 2019

@linusjf
Copy link
Author

linusjf commented Sep 23, 2019

https://coderanch.com/t/708483/java/understanding-implicit-parameter

This thread has a great explanation of this keyword right upto receive parameter.

@linusjf
Copy link
Author

linusjf commented Sep 23, 2019

@linusjf
Copy link
Author

linusjf commented Oct 3, 2019

https://github.com/Fernal73/LearnJava/blob/master/Reflection/reflection/SyntheticSample.java

A synthetic method in an inner class displays it's signature as consisting of a call to a receiver parameter.

@adangel
Copy link
Member

adangel commented Oct 3, 2019

FYI - I've updated to description to have two examples: one the new rule should detect (e.g. "invalid code") and another one, that should produce no violation (e.g. "valid code")

@linusjf
Copy link
Author

linusjf commented Oct 3, 2019 via email

@linusjf
Copy link
Author

linusjf commented Nov 17, 2020

Is this going to be in PMD 7?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:new-rule Proposal to add a new built-in rule
Projects
None yet
Development

No branches or pull requests

3 participants