Discussion:
Confusing error message for inner non-public service provider
Gunnar Morling
2017-02-06 21:33:36 UTC
Permalink
Hi,

I have a service provider which is a public static inner class of an
outer class with default visibility.

As per the ServiceLoader docs, service providers must be public
classes, so this provider is rightfully rejected by javac when
referenced in module-info.java. But the error message is rather
confusing:

error: package com.example.internal is not visible
provides com.example.SomeService with
com.example.internal.Outer.ServiceImpl;
^
(package com.example.internal is declared in module com.example,
but module com.example does not read it)
error: the service implementation does not have a default
constructor: ServiceImpl
provides com.example.SomeService with com.example.internal.
Outer.ServiceImpl

ServiceImpl declares no explicit constructor, so there should be a
default constructor. But also the referral to package visibility seems
odd. In contrast, if defining the provider in a non-inner class with
default visibility, the error message is more what I'd have expected:

error: ServiceImpl is not public in com.example.internal; cannot
be accessed from outside package
provides com.example.SomeService with com.example.internal.ServiceImpl;

Surely it's an edge case, but could the error message for the former
case be adjusted to look more like in the latter?

Thanks,

--Gunnar
Alex Buckley
2017-02-06 22:05:14 UTC
Permalink
Post by Gunnar Morling
I have a service provider which is a public static inner class of an
outer class with default visibility.
I think you mean public static _nested_ class, since an inner class
isn't static by definition.

Also I think you mean default (package) access. Visibility is something
else -- see the draft JLS changes in the JSR 376 EDR.
Post by Gunnar Morling
As per the ServiceLoader docs, service providers must be public
classes, so this provider is rightfully rejected by javac when
referenced in module-info.java. But the error message is rather
error: package com.example.internal is not visible
provides com.example.SomeService with
com.example.internal.Outer.ServiceImpl;
^
(package com.example.internal is declared in module com.example,
but module com.example does not read it)
error: the service implementation does not have a default
constructor: ServiceImpl
provides com.example.SomeService with com.example.internal.
Outer.ServiceImpl
ServiceImpl declares no explicit constructor, so there should be a
default constructor.
Please see
http://cr.openjdk.java.net/~mr/jigsaw/spec/lang-vm.html#jigsaw-1.1.4 to
understand the rules for provider constructors.
Post by Gunnar Morling
But also the referral to package visibility seems odd.
It really means visibility, not accessibility. A Java compiler must now
effect visibility in a similar way to class loaders. javac is telling
you that no compilation units of the package com.example.internal are
visible to your module. That is nothing to do with 'public', package
access, 'protected', or 'private'.
Post by Gunnar Morling
In contrast, if defining the provider in a non-inner class with
error: ServiceImpl is not public in com.example.internal; cannot
be accessed from outside package
provides com.example.SomeService with com.example.internal.ServiceImpl;
Again, you mean default (package) access. The error is correct of course.
Post by Gunnar Morling
Surely it's an edge case, but could the error message for the former
case be adjusted to look more like in the latter?
The two error messages for the former are "right". The second message
could be clarified to set out the requirement for an explicit
constructor in lieu of an explicit provider() method.

Alex
Gunnar Morling
2017-02-07 09:11:45 UTC
Permalink
Hi Alex,

Yes, I meant a nested class and default access, sorry for not being
precise with the terms. These are the concerned types:

---
package com.example;
public interface SomeService {
public void foo();
}
---
package com.example.internal;
class Outer {

public static class ServiceImpl implements com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
}
---
package com.example.internal;
class ServiceImpl implements com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
---
module com.example {
exports com.example;
provides com.example.SomeService with com.example.internal.ServiceImpl;
provides com.example.SomeService with
com.example.internal.Outer.ServiceImpl;
}
---

Essentially, I'm wondering:

* Why Outer.ServiceImpl triggers the error about package visibility
while ServiceImpl doesn't (I had a look at the EDR JLS, but I couldn't
find an explanation for that, happy about any specific pointers).
* Why Outer.ServiceImpl triggers "does not have a default constructor"
(ServiceImpl does not). Maybe a hint would be nice that is caused by
Outer not having public access.

--Gunnar
Post by Gunnar Morling
I have a service provider which is a public static inner class of an
outer class with default visibility.
I think you mean public static _nested_ class, since an inner class isn't
static by definition.
Also I think you mean default (package) access. Visibility is something else
-- see the draft JLS changes in the JSR 376 EDR.
Post by Gunnar Morling
As per the ServiceLoader docs, service providers must be public
classes, so this provider is rightfully rejected by javac when
referenced in module-info.java. But the error message is rather
error: package com.example.internal is not visible
provides com.example.SomeService with
com.example.internal.Outer.ServiceImpl;
^
(package com.example.internal is declared in module com.example,
but module com.example does not read it)
error: the service implementation does not have a default
constructor: ServiceImpl
provides com.example.SomeService with com.example.internal.
Outer.ServiceImpl
ServiceImpl declares no explicit constructor, so there should be a
default constructor.
Please see
http://cr.openjdk.java.net/~mr/jigsaw/spec/lang-vm.html#jigsaw-1.1.4 to
understand the rules for provider constructors.
Post by Gunnar Morling
But also the referral to package visibility seems odd.
It really means visibility, not accessibility. A Java compiler must now
effect visibility in a similar way to class loaders. javac is telling you
that no compilation units of the package com.example.internal are visible to
your module. That is nothing to do with 'public', package access,
'protected', or 'private'.
Post by Gunnar Morling
In contrast, if defining the provider in a non-inner class with
error: ServiceImpl is not public in com.example.internal; cannot
be accessed from outside package
provides com.example.SomeService with
com.example.internal.ServiceImpl;
Again, you mean default (package) access. The error is correct of course.
Post by Gunnar Morling
Surely it's an edge case, but could the error message for the former
case be adjusted to look more like in the latter?
The two error messages for the former are "right". The second message could
be clarified to set out the requirement for an explicit constructor in lieu
of an explicit provider() method.
Alex
Alex Buckley
2017-02-07 19:21:41 UTC
Permalink
Post by Gunnar Morling
---
package com.example;
public interface SomeService {
public void foo();
}
---
package com.example.internal;
class Outer {
public static class ServiceImpl implements com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
}
---
package com.example.internal;
class ServiceImpl implements com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
---
module com.example {
exports com.example;
provides com.example.SomeService with com.example.internal.ServiceImpl;
provides com.example.SomeService with
com.example.internal.Outer.ServiceImpl;
}
---
* Why Outer.ServiceImpl triggers the error about package visibility
while ServiceImpl doesn't (I had a look at the EDR JLS, but I couldn't
find an explanation for that, happy about any specific pointers).
* Why Outer.ServiceImpl triggers "does not have a default constructor"
(ServiceImpl does not). Maybe a hint would be nice that is caused by
Outer not having public access.
Thanks for showing the code. Since everything in the same module,
package visibility is not relevant and javac shouldn't mention it.

I suspect that javac is getting tripped up by the fact that
Outer.ServiceImpl is declared 'public' (as the JLS and ServiceLoader
both demand) but it isn't widely accessible, even within the com.example
module, due to Outer's default (package) access. I believe the JLS and
ServiceLoader rules are correct, so it's a javac bug.

Alex
Vicente Romero
2017-02-09 22:49:08 UTC
Permalink
Hi Alex,

Just to double check, the right javac behavior in this case should be to
issue similar errors in both cases like:

some position here: error: ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
some other position here: error: Outer.ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package

without mentioning in any case anything about visibility right?

Thanks,
Vicente
Post by Alex Buckley
Post by Gunnar Morling
---
package com.example;
public interface SomeService {
public void foo();
}
---
package com.example.internal;
class Outer {
public static class ServiceImpl implements
com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
}
---
package com.example.internal;
class ServiceImpl implements com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
---
module com.example {
exports com.example;
provides com.example.SomeService with
com.example.internal.ServiceImpl;
provides com.example.SomeService with
com.example.internal.Outer.ServiceImpl;
}
---
* Why Outer.ServiceImpl triggers the error about package visibility
while ServiceImpl doesn't (I had a look at the EDR JLS, but I couldn't
find an explanation for that, happy about any specific pointers).
* Why Outer.ServiceImpl triggers "does not have a default constructor"
(ServiceImpl does not). Maybe a hint would be nice that is caused by
Outer not having public access.
Thanks for showing the code. Since everything in the same module,
package visibility is not relevant and javac shouldn't mention it.
I suspect that javac is getting tripped up by the fact that
Outer.ServiceImpl is declared 'public' (as the JLS and ServiceLoader
both demand) but it isn't widely accessible, even within the
com.example module, due to Outer's default (package) access. I believe
the JLS and ServiceLoader rules are correct, so it's a javac bug.
Alex
Alex Buckley
2017-02-09 23:07:40 UTC
Permalink
Post by Vicente Romero
Just to double check, the right javac behavior in this case should be to
some position here: error: ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
It's correct to give an error, but the clause "cannot be accessed from
outside package" should be dropped (it's not relevant to ServiceLoader).
Post by Vicente Romero
some other position here: error: Outer.ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
It's not correct to give an error at all. The JLS (acting on behalf of
ServiceLoader) is not interested in the class Outer.ServiceImpl being
"accessible" by some arbitrary client. All the JLS wants is for the
class to be 'public'.
Post by Vicente Romero
without mentioning in any case anything about visibility right?
Correct. All the types we're discussing are in the same module so they
(and their packages) are all visible to each other; package visibility
is irrelevant to this example.

Alex
Vicente Romero
2017-02-09 23:12:11 UTC
Permalink
Hi Alex,

Thanks for your answer,
Vicente
Post by Alex Buckley
Post by Vicente Romero
Just to double check, the right javac behavior in this case should be to
some position here: error: ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
It's correct to give an error, but the clause "cannot be accessed from
outside package" should be dropped (it's not relevant to ServiceLoader).
Post by Vicente Romero
some other position here: error: Outer.ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
It's not correct to give an error at all. The JLS (acting on behalf of
ServiceLoader) is not interested in the class Outer.ServiceImpl being
"accessible" by some arbitrary client. All the JLS wants is for the
class to be 'public'.
Post by Vicente Romero
without mentioning in any case anything about visibility right?
Correct. All the types we're discussing are in the same module so they
(and their packages) are all visible to each other; package visibility
is irrelevant to this example.
Alex
Jonathan Gibbons
2017-02-10 00:21:56 UTC
Permalink
All the JLS wants is for the class to be 'public'.
Alex:

Does that just apply locally to the declaration of the class itself, or
does it also indirectly apply to any enclosing classes, in the case of a
nested class?

-- Jon
Alex Buckley
2017-02-10 01:23:30 UTC
Permalink
Post by Jonathan Gibbons
All the JLS wants is for the class to be 'public'.
Does that just apply locally to the declaration of the class itself, or
does it also indirectly apply to any enclosing classes, in the case of a
nested class?
Just the declaration of the class itself. The JLS does NOT want the
specified class must be accessible from <some other class>, which would
imply a chain of accessibility from <some other class> to the provider
class. The JLS just wants the 'public' modifier on the class
declaration, end of story.

Alex
Alex Buckley
2017-02-10 01:31:30 UTC
Permalink
// Rewording and resending to avoid confusion.
Post by Jonathan Gibbons
All the JLS wants is for the class to be 'public'.
Does that just apply locally to the declaration of the class itself, or
does it also indirectly apply to any enclosing classes, in the case of a
nested class?
Just the declaration of the class itself. The JLS does NOT want the
specified class to be accessible from <some other class>. That is, the
JLS does not care about a chain of access from <some other class> to the
provider class, which might conceivably allow the provider class to have
default (package) access. The JLS just wants the 'public' modifier on
the class declaration, end of story.

Alex
Peter Levart
2017-02-13 17:08:00 UTC
Permalink
Hi,

Just wanted to note that the confusing javac error message is not
specific to services and service providers.

Take the following example:

src/moda/module-info.java:

module moda {
exports pkga;
}

src/moda/pkga/Outer.java:

package pkga;
class Outer {
public static class Untouchable {
public static void touch() {
throw new AssertionError("Can't touch this!");
}
}
}

src/modb/module-info.java:

module modb {
requires moda;
}

src/modb/pkgb/Intruder.java:

package pkgb;
public class Intruder {
public static void main(String[] args) {
pkga.Outer.Untouchable.touch();
}
}


$ javac -d out --module-path out --module-source-path src `find src
-name '*.java'`
src/modb/pkgb/Intruder.java:6: error: package pkga is not visible
pkga.Outer.Untouchable.touch();
^
(package pkga is declared in module moda, which does not export it to
module modb)
1 error



Regards, Peter
Post by Gunnar Morling
Hi Alex,
Just to double check, the right javac behavior in this case should be
some position here: error: ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
some other position here: error: Outer.ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
without mentioning in any case anything about visibility right?
Thanks,
Vicente
Post by Alex Buckley
Post by Gunnar Morling
---
package com.example;
public interface SomeService {
public void foo();
}
---
package com.example.internal;
class Outer {
public static class ServiceImpl implements
com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
}
---
package com.example.internal;
class ServiceImpl implements com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
---
module com.example {
exports com.example;
provides com.example.SomeService with
com.example.internal.ServiceImpl;
provides com.example.SomeService with
com.example.internal.Outer.ServiceImpl;
}
---
* Why Outer.ServiceImpl triggers the error about package visibility
while ServiceImpl doesn't (I had a look at the EDR JLS, but I couldn't
find an explanation for that, happy about any specific pointers).
* Why Outer.ServiceImpl triggers "does not have a default constructor"
(ServiceImpl does not). Maybe a hint would be nice that is caused by
Outer not having public access.
Thanks for showing the code. Since everything in the same module,
package visibility is not relevant and javac shouldn't mention it.
I suspect that javac is getting tripped up by the fact that
Outer.ServiceImpl is declared 'public' (as the JLS and ServiceLoader
both demand) but it isn't widely accessible, even within the
com.example module, due to Outer's default (package) access. I
believe the JLS and ServiceLoader rules are correct, so it's a javac
bug.
Alex
Jonathan Gibbons
2017-02-13 19:10:53 UTC
Permalink
Peter,

The circumstances of the analysis of service providers are very
different to the normal JLS rules regarding visibility and accessibility
of named items.

I know that we have some cleanup to do, for javac to "catch up" with the
latest updates to JLS, especially with regards to the difference between
"visibility" and "accessibility". Is that the issue you concerned
about, or is there something else about this message you find confusing.

-- Jon
Post by Peter Levart
Hi,
Just wanted to note that the confusing javac error message is not
specific to services and service providers.
module moda {
exports pkga;
}
package pkga;
class Outer {
public static class Untouchable {
public static void touch() {
throw new AssertionError("Can't touch this!");
}
}
}
module modb {
requires moda;
}
package pkgb;
public class Intruder {
public static void main(String[] args) {
pkga.Outer.Untouchable.touch();
}
}
$ javac -d out --module-path out --module-source-path src `find src
-name '*.java'`
src/modb/pkgb/Intruder.java:6: error: package pkga is not visible
pkga.Outer.Untouchable.touch();
^
(package pkga is declared in module moda, which does not export it
to module modb)
1 error
Regards, Peter
Post by Gunnar Morling
Hi Alex,
Just to double check, the right javac behavior in this case should be
some position here: error: ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
some other position here: error: Outer.ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
without mentioning in any case anything about visibility right?
Thanks,
Vicente
Post by Alex Buckley
Post by Gunnar Morling
---
package com.example;
public interface SomeService {
public void foo();
}
---
package com.example.internal;
class Outer {
public static class ServiceImpl implements
com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
}
---
package com.example.internal;
class ServiceImpl implements com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
---
module com.example {
exports com.example;
provides com.example.SomeService with
com.example.internal.ServiceImpl;
provides com.example.SomeService with
com.example.internal.Outer.ServiceImpl;
}
---
* Why Outer.ServiceImpl triggers the error about package visibility
while ServiceImpl doesn't (I had a look at the EDR JLS, but I couldn't
find an explanation for that, happy about any specific pointers).
* Why Outer.ServiceImpl triggers "does not have a default constructor"
(ServiceImpl does not). Maybe a hint would be nice that is caused by
Outer not having public access.
Thanks for showing the code. Since everything in the same module,
package visibility is not relevant and javac shouldn't mention it.
I suspect that javac is getting tripped up by the fact that
Outer.ServiceImpl is declared 'public' (as the JLS and ServiceLoader
both demand) but it isn't widely accessible, even within the
com.example module, due to Outer's default (package) access. I
believe the JLS and ServiceLoader rules are correct, so it's a javac
bug.
Alex
Peter Levart
2017-02-14 08:00:47 UTC
Permalink
Hi Jon,

I understand that service providers have a different set of rules (they
are usually not universally accessible classes) and therefore the
requirement for them to have public access modifier is merely a choice
JLS made to enforce a certain degree of strictness. Classes with default
(package-private) modifier could easily be allowed too, but I guess some
strictness is welcome. I just wanted to share what I observed: that the
javac error message is similar also when trying to access a public
nested class in a package-private enclosing class in an exported
package, from a different module, which has nothing to do with service
providers.

Regards, Peter
Post by Jonathan Gibbons
Peter,
The circumstances of the analysis of service providers are very
different to the normal JLS rules regarding visibility and
accessibility of named items.
I know that we have some cleanup to do, for javac to "catch up" with
the latest updates to JLS, especially with regards to the difference
between "visibility" and "accessibility". Is that the issue you
concerned about, or is there something else about this message you
find confusing.
-- Jon
Post by Peter Levart
Hi,
Just wanted to note that the confusing javac error message is not
specific to services and service providers.
module moda {
exports pkga;
}
package pkga;
class Outer {
public static class Untouchable {
public static void touch() {
throw new AssertionError("Can't touch this!");
}
}
}
module modb {
requires moda;
}
package pkgb;
public class Intruder {
public static void main(String[] args) {
pkga.Outer.Untouchable.touch();
}
}
$ javac -d out --module-path out --module-source-path src `find src
-name '*.java'`
src/modb/pkgb/Intruder.java:6: error: package pkga is not visible
pkga.Outer.Untouchable.touch();
^
(package pkga is declared in module moda, which does not export it
to module modb)
1 error
Regards, Peter
Post by Gunnar Morling
Hi Alex,
Just to double check, the right javac behavior in this case should
some position here: error: ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
some other position here: error: Outer.ServiceImpl is not public in
com.example.internal; cannot be accessed from outside package
without mentioning in any case anything about visibility right?
Thanks,
Vicente
Post by Alex Buckley
Post by Gunnar Morling
---
package com.example;
public interface SomeService {
public void foo();
}
---
package com.example.internal;
class Outer {
public static class ServiceImpl implements
com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
}
---
package com.example.internal;
class ServiceImpl implements com.example.SomeService {
public ServiceImpl() {}
public void foo() {}
}
---
module com.example {
exports com.example;
provides com.example.SomeService with
com.example.internal.ServiceImpl;
provides com.example.SomeService with
com.example.internal.Outer.ServiceImpl;
}
---
* Why Outer.ServiceImpl triggers the error about package visibility
while ServiceImpl doesn't (I had a look at the EDR JLS, but I couldn't
find an explanation for that, happy about any specific pointers).
* Why Outer.ServiceImpl triggers "does not have a default
constructor"
(ServiceImpl does not). Maybe a hint would be nice that is caused by
Outer not having public access.
Thanks for showing the code. Since everything in the same module,
package visibility is not relevant and javac shouldn't mention it.
I suspect that javac is getting tripped up by the fact that
Outer.ServiceImpl is declared 'public' (as the JLS and
ServiceLoader both demand) but it isn't widely accessible, even
within the com.example module, due to Outer's default (package)
access. I believe the JLS and ServiceLoader rules are correct, so
it's a javac bug.
Alex
Loading...