Confusing error message for inner non-public service provider

[Gunnar Morling]

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



2017-02-06 23:05 GMT+01:00 Alex Buckley <alex.buckley at oracle.com>:
> 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