Confusing error message for inner non-public service provider

[Alex Buckley]

On 2/6/2017 1:33 PM, Gunnar Morling wrote:
> 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.

> 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.

Please see 
http://cr.openjdk.java.net/~mr/jigsaw/spec/lang-vm.html#jigsaw-1.1.4 to 
understand the rules for provider constructors.

> 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'.

> 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;

Again, you mean default (package) access. The error is correct of course.

> 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