error message about not initialized field is not correct

[indika-dev]

Hi,

in the following snippet, I get the following error message at line return nrOfNodesPerPart.getOrDefault(partsMatcher.group(), Long.valueOf(0l)).longValue(); :

The blank final field nrOfNodesPerPart may not have been initialized [33554513]

import java.util.stream.Collector;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
public class MergePatchesCollector
    implements Collector<String, List<String>, List<String>> {
  private final static Pattern partsPattern =
      Pattern.compile("(provides|internal|external|softwareDependencies)");

  private final Map<String, Long> nrOfNodesPerPart;

  private Function<String, Long> getNrOfNodesForPart = (path) -> {
    if (path == null || path.isEmpty() || path.isBlank()) {
      return 0L;
    }
    Matcher partsMatcher = partsPattern.matcher(path);
    if (partsMatcher.find()) {
      return nrOfNodesPerPart.getOrDefault(partsMatcher.group(), Long.valueOf(0l)).longValue();
    }
    return 0L;
  };
}

If I rewrite as a function, the error message disappears:

import java.util.stream.Collector;
import java.util.List;
import java.util.Map;

import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
public class MergePatchesCollector
    implements Collector<String, List<String>, List<String>> {
  private final static Pattern partsPattern =
      Pattern.compile("(provides|internal|external|softwareDependencies)");

  private final Map<String, Long> nrOfNodesPerPart;

  private long getNrOfNodesForPart(String path) {
    if (path == null || path.isEmpty() || path.isBlank()) {
      return 0L;
    }
    Matcher partsMatcher = partsPattern.matcher(path);
    if (partsMatcher.find()) {
      return nrOfNodesPerPart.getOrDefault(partsMatcher.group(), 0L);
    }
    return 0L;
  };
}

In my opinion, the error message in the first case doesn't make sense, because the field is initialized via the constructor or do I oversee something?

[fbricon]

looks like an upstream bug from the Eclipse compiler. @snjeza can you confirm?

[snjeza]

I can reproduce the issue in the jdt master branch and javac.

jdt returns

The blank final field nrOfNodesPerPart may not have been initialized

javac returns

variable nrOfNodesPerPart might not have been initialized

[indika-dev]

I can confirm, that javac returns that error. But I don't understand, why this error message appears. I tested with an explicitly written constructor(in case lombok is not working correctly), but the error message remains the same.
Should I open a bug report somewhere else?

[0dinD]

I don't think this is a bug at all.

You need to initialize the nrOfNodesPerPart field before you can use it. You are using it when you initialize the getNrOfNodesForPart field, because, According to the JLS:

Unlike code appearing in anonymous class declarations, the meaning of names and the this and super keywords appearing in a lambda body, along with the accessibility of referenced declarations, are the same as in the surrounding context (except that lambda parameters introduce new names).

The transparency of this (both explicit and implicit) in the body of a lambda expression - that is, treating it the same as in the surrounding context - allows more flexibility for implementations, and prevents the meaning of unqualified names in the body from being dependent on overload resolution.

Practically speaking, it is unusual for a lambda expression to need to talk about itself (either to call itself recursively or to invoke its other methods), while it is more common to want to use names to refer to things in the enclosing class that would otherwise be shadowed (this, toString()). If it is necessary for a lambda expression to refer to itself (as if via this), a method reference or an anonymous inner class should be used instead.

So when you are using nrOfNodesPerPart, i.e. this.nrOfNodesPerPart, it is not actually evaluated in the context of the lambda body, but rather, the surrounding context, i.e. the field initializer for getNrOfNodesForPart. And at that point, the nrOfNodesPerPart field hasn't been initialized yet, since the constructor runs after the initializers. So you get a compile error.

At least, that's my analysis of the situation, based on what I've read online 🙂. Take it with a grain of salt, I'm not a JLS expert.

You have already found a solution to the problem (using a method instead of a lambda in a field). Here are a few other potential solutions I could think of:

1: Initialize nrOfNodesPerPart before getNrOfNodesForPart (using field initialization):

import java.util.stream.Collector;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
public class MergePatchesCollector
    implements Collector<String, List<String>, List<String>> {
  private final static Pattern partsPattern =
      Pattern.compile("(provides|internal|external|softwareDependencies)");

  private final Map<String, Long> nrOfNodesPerPart = null; // TODO: Initialize to a better value?

  private Function<String, Long> getNrOfNodesForPart = (path) -> {
    if (path == null || path.isEmpty() || path.isBlank()) {
      return 0L;
    }
    Matcher partsMatcher = partsPattern.matcher(path);
    if (partsMatcher.find()) {
      return nrOfNodesPerPart.getOrDefault(partsMatcher.group(), Long.valueOf(0l)).longValue();
    }
    return 0L;
  };
}

2: Initialize nrOfNodesPerPart before getNrOfNodesForPart (using constructor initialization):

import java.util.stream.Collector;
import java.util.List;
import java.util.Map;
import java.util.function.Function;

public class MergePatchesCollector
    implements Collector<String, List<String>, List<String>> {
  private final static Pattern partsPattern =
      Pattern.compile("(provides|internal|external|softwareDependencies)");

  private final Map<String, Long> nrOfNodesPerPart;

  private Function<String, Long> getNrOfNodesForPart;

  public MergePatchesCollector(Map<String, Long> nrOfNodesPerPart) {
    this.nrOfNodesPerPart = nrOfNodesPerPart;
    this.getNrOfNodesForPart = (path) -> {
      if (path == null || path.isEmpty() || path.isBlank()) {
        return 0L;
      }
      Matcher partsMatcher = partsPattern.matcher(path);
      if (partsMatcher.find()) {
        return this.nrOfNodesPerPart.getOrDefault(partsMatcher.group(), Long.valueOf(0l)).longValue();
      }
      return 0L;
    };
  }

}

3: Use an anonymous class instead (where this has a different meaning):

import java.util.stream.Collector;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
public class MergePatchesCollector
    implements Collector<String, List<String>, List<String>> {
  private final static Pattern partsPattern =
      Pattern.compile("(provides|internal|external|softwareDependencies)");

  private final Map<String, Long> nrOfNodesPerPart;

  private Function<String, Long> getNrOfNodesForPart = new Function<String, Long>() {
    @Override
    public Long apply(String path) {
      if (path == null || path.isEmpty() || path.isBlank()) {
        return 0L;
      }
      Matcher partsMatcher = partsPattern.matcher(path);
      if (partsMatcher.find()) {
        return nrOfNodesPerPart.getOrDefault(partsMatcher.group(), Long.valueOf(0l)).longValue();
      }
      return 0L;
    }
  };
}

4: Use a method, but also create a field using a method reference (if the field was important to you):

import java.util.stream.Collector;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
public class MergePatchesCollector
    implements Collector<String, List<String>, List<String>> {
  private final static Pattern partsPattern =
      Pattern.compile("(provides|internal|external|softwareDependencies)");

  private final Map<String, Long> nrOfNodesPerPart;

  private Function<String, Long> getNrOfNodesForPart = this::getNrOfNodesForPart;

  private long getNrOfNodesForPart(String path) {
    if (path == null || path.isEmpty() || path.isBlank()) {
      return 0L;
    }
    Matcher partsMatcher = partsPattern.matcher(path);
    if (partsMatcher.find()) {
      return nrOfNodesPerPart.getOrDefault(partsMatcher.group(), 0L);
    }
    return 0L;
  }
}

5: Bypass compiler checks using a local variable:

import java.util.stream.Collector;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
public class MergePatchesCollector
    implements Collector<String, List<String>, List<String>> {
  private final static Pattern partsPattern =
      Pattern.compile("(provides|internal|external|softwareDependencies)");

  private final Map<String, Long> nrOfNodesPerPart;

  private Function<String, Long> getNrOfNodesForPart = (path) -> {
    if (path == null || path.isEmpty() || path.isBlank()) {
      return 0L;
    }
    Matcher partsMatcher = partsPattern.matcher(path);
    if (partsMatcher.find()) {
      MergePatchesCollector mpc = this;
      // this.nrOfNodesPerPart is still potentially uninitialized, but the compiler doesn't understand that mpc == this
      return mpc.nrOfNodesPerPart.getOrDefault(partsMatcher.group(), Long.valueOf(0l)).longValue();
    }
    return 0L;
  };
}

[indika-dev]

Wow! Thank you very much for your very detailed answer, @0dinD ! Your excerpt explains it very good, why this is not a bug. I had this suspicion, that this is not a bug, but I really couldn't explain it for myself. But yes, this in a lambda expression is somewhat different than using it in the surrounding context. I don't need a grain of salt, it's pretty good defined, so I have to follow the definition. :)
I will stick to my solution with the method, because it has the best fit in this situation. I was thinking about adding nrOfNodesPerPart as a parameter to the lambda expression, but not everything has to be a function. ;)