Confusing error message for inner non-public service provider

[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

On 02/13/2017 09:08 AM, Peter Levart wrote:
> 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
>
>
> On 02/09/2017 11:49 PM, Vicente Romero wrote:
>> 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
>>
>>
>> On 02/07/2017 02:21 PM, Alex Buckley wrote:
>>> On 2/7/2017 1:11 AM, Gunnar Morling wrote:
>>>>      ---
>>>>      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.
>>>
>>> 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
>>
>