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

Method reference for constructor produces weird error with @UnderInitialization #3637

Closed
vlsi opened this issue Sep 7, 2020 · 3 comments
Closed

Comments

@vlsi
Copy link
Contributor

vlsi commented Sep 7, 2020

public interface DirectedGraph<V, E> {
  interface EdgeFactory<V, E> {
    E createEdge(V v0, V v1);
  }
}

public class DefaultEdge {
  public final Object source;
  public final Object target;

  public DefaultEdge(Object source, Object target) {
    this.source = Objects.requireNonNull(source);
    this.target = Objects.requireNonNull(target);
  }

  public static <V> DirectedGraph.EdgeFactory<V, DefaultEdge> factory() {
    return DefaultEdge::new; // <-- error here
  }
}
calcite/core/src/main/java/org/apache/calcite/util/graph/DefaultEdge.java:47: error: [methodref.param.invalid] Incompatible parameter type for source
    return DefaultEdge::new;
           ^
  Method
    @UnderInitialization(java.lang.Object.class) @NonNull DefaultEdge <init>(@Initialized @NonNull Object p0, @Initialized @NonNull Object p1) in org.apache.calcite.util.graph.DefaultEdge
  is not a valid method reference for
    @Initialized @NonNull DefaultEdge createEdge(@Initialized @NonNull EdgeFactory<V extends @Initialized @Nullable Object, @Initialized @NonNull DefaultEdge> this, V extends @Initialized @Nullable Object p0, V extends @Initialized @Nullable Object p1) in org.apache.calcite.util.graph.DirectedGraph.EdgeFactory
  found   : @Initialized @NonNull Object
  required: V extends @Initialized @Nullable Object
@mernst
Copy link
Member

mernst commented Oct 20, 2020

Thanks for the small test case.

The essence of the error is:

  Incompatible parameter type for source
  ...
  Method
    DefaultEdge <init>(@NonNull Object p0) in DefaultEdge
  is not a valid method reference for
    Object createEdge(V extends @Nullable Object p0) in DirectedGraph.EdgeFactory
  found   : @Initialized @NonNull Object
  required: V extends @Initialized @Nullable Object

This indicates a mismatch between

  • the declaration of the DefaultEdge constructor and
  • the declaration of createEdge()

Here is a simplified version of your test case (with an import statement added so that it compiles):

import java.util.Objects;

public interface DirectedGraph<V> {
  interface EdgeFactory<V> {
    Object createEdge(V v0);
  }
}

class DefaultEdge {
  public final Object source;

  public DefaultEdge(Object sourceArg) {
    this.source = Objects.requireNonNull(sourceArg);
  }

  public static <V> DirectedGraph.EdgeFactory<V> factory() {
    return DefaultEdge::new; // <-- error here
  }
}

createEdge is declared to take any argument whatsoever, including a @Nullable one.

factory() instantiates EdgeFactory arbitrarily. However, the DefaultEdge constructor is declared to take a non-null argument (@NonNull is the default).

To make the code type-check, you can change the annotations to better express the contract:

import org.checkerframework.checker.nullness.qual.NonNull;
import java.util.Objects;

public interface DirectedGraph<V> {
  interface EdgeFactory<V> {
    Object createEdge(V v0);
  }
}

class DefaultEdge {
  public final Object source;

  public DefaultEdge(Object sourceArg) {
    this.source = Objects.requireNonNull(sourceArg);
  }

  // added "extends @NonNull Object" on line below
  public static <V extends @NonNull Object> DirectedGraph.EdgeFactory<V> factory() {
    return DefaultEdge::new;
  }
}

I agree the error message is a bit hard to interpret. Let us know if you have a suggestion for clarifying it.

@vlsi
Copy link
Contributor Author

vlsi commented Oct 20, 2020

It looks like I somehow fixed the error because the current code is indeed <V extends Object>.

It might be a case of an unfortunate naming. The parameter name is source, so the message was like Incompatible parameter type for source, and I might confuse source to mean something like source method.

What do you think of moving found/required before the full type signatures?

@vlsi vlsi closed this as completed Oct 20, 2020
@mernst
Copy link
Member

mernst commented Oct 21, 2020

What do you think of moving found/required before the full type signatures?

This is a good idea. Thanks. I will open a pull request with that change.

mernst added a commit to mernst/checker-framework that referenced this issue Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants