Pattern variable and field shadowing

[Gavin Bierman]

Hi Tagir, 

No, that’s very much the expected behaviour. The idea of flow scoping is that a pattern variable is only *in scope* where it has definitely matched. Thus, in places where it is definitely not matched - like your else block - it is not in scope. Thus it doesn’t exist. Thus if there is field with that name, then that is the meaning of the name.

We looked long and hard at exactly this example; as Brian mentioned, it is called the “Swiss cheese” problem in the team! The problem is that not doing this also comes at a heavy cost. For example, we could say that the variable str in your example is in scope but not DA, for example. But that means that using a pattern variable poisons it forever more in the statement/block. This forbids very, very common code like:

if (a instanceof Point p) {
…
} else {
    if (b instanceof Point p) { // Can’t use p even though it won’t be bound
        …
    } 
}

In other words, you’ll have to find a new pattern variable everytime. [And this gets even worse in switch.] That’s a real pain, and I think most people will say “why do I have to find a new name, can’t you figure it out?”. Yes, we could, but then we’d have to do something even more special case, like allowing shadowing of pattern variables with pattern variables. But we’re now unravelling all sorts of aspects of the way Java deals with naming, and that feels like something that most users will never have in their heads. We’re hoping the “only in scope where it is matched” is a simpler rule that is easier to internalize.

That’s not to say that there couldn’t be tooling to provide warnings in the case where pattern variables shadow fields (as it inevitably means that there will be some holes in the cheese), hint hint :-)

But please use the preview feature and let us know how you get along so we can assess this design decision.

Thanks,
Gavin


> On 28 Nov 2019, at 03:39, Tagir Valeev <amaembo at gmail.com> wrote:
> 
> Hello!
> 
> Consider the following code:
> 
> public class A {
>    private String str;
> 
>    public void f(Object obj) {
>        if (obj instanceof String str) {
>            System.out.println(str.toLowerCase()); // str refers to
> pattern binding
>        } else {
>            System.out.println(str.toLowerCase()); // str refers to the field
>        }
>    }
> }
> 
> I thought that such a code should be rejected by the compiler, as it's
> confusing and could be a source of very subtle bugs. However, I
> haven't found any explicit statement regarding this in the latest spec
> draft [1]. Could you please clarify whether such code is acceptable
> and point me to the relevant part of the spec draft. Thank you!
> 
> With best regards,
> Tagir Valeev.
> 
> [1] http://cr.openjdk.java.net/~gbierman/jep305/jep305-20191021/specs/patterns-instanceof-jls.html