RFR 8189749: Devise strategy for making source level checks more uniform

[Maurizio Cimadamore]

Thanks,
I will make the change ahead of the push (w/o submitting another round) 
if that's ok

Maurizio


On 02/12/17 00:29, Jonathan Gibbons wrote:
> +1. Very nice.
>
> Nit: I would suggest changing multi-catch statements to the plural form.
>
> tests/MulticatchNotSupported.java:39: error: multi-catch statement is 
> not supported in -source 6
>
> -- Jon
>
> On 11/28/2017 06:19 AM, Maurizio Cimadamore wrote:
>> Updated webrev here:
>>
>> http://cr.openjdk.java.net/~mcimadamore/8189749-v2/
>>
>> I've included a report of the diagnostics that have been tweaked by 
>> this webrev here to make sure that singular vs. plural is used 
>> effectively:
>>
>> http://cr.openjdk.java.net/~mcimadamore/8189749-v2/diags.html
>>
>> Cheers
>> Maurizio
>>
>>
>> On 20/10/17 19:07, Maurizio Cimadamore wrote:
>>> Hi,
>>> this biggie patch allows the treatment of source level checks to be 
>>> more uniform. The general problem is explained in details here:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8189749
>>>
>>> Webrev is here:
>>>
>>> http://cr.openjdk.java.net/~mcimadamore/8189749/
>>>
>>> The way I approached this, was to create a first class entity, an 
>>> enum called Feature. Each 'feature' has:
>>>
>>> * a min source level (e.g. what is the min source level required to 
>>> compile that feature, meaning that lower levels will result in errors)
>>> * a max source level (e.g. what is the max source level required to 
>>> compile that feature, meaning that higher levels will result in errors)
>>> * an optional resource key
>>>
>>> The first two properties are obviously used to decided if feature 
>>> XYZ can be used given source level N. The last property is used to 
>>> automate generation of source check diagnostics. Note that this 
>>> patch introduce a _single_ source check diagnostic:
>>>
>>> # 0: message segment (feature), 1: string (found version), 2: string 
>>> (expected version)
>>> compiler.misc.feature.not.supported.in.source=\
>>>    {0} not supported in -source {1}\n\
>>>     (use -source {2} or higher to enable {0})
>>>
>>> And then there's a bunch of feature fragments, example:
>>>
>>> compiler.misc.feature.modules=\
>>>     modules
>>>
>>> compiler.misc.feature.diamond.and.anon.class=\
>>>     ''<>'' with anonymous inner classes
>>>
>>> compiler.misc.feature.binary.lit=\
>>>     binary literals
>>>
>>> (and many more :-))
>>>
>>> Since each feature 'knows' its fragment, it is super easy for the 
>>> compiler to generate the source level check diagnostic for any 
>>> feature. And, if you need to add a new feature, you just need to add 
>>> an enum value, and wired it up with the corresponding resource key - 
>>> done :-)
>>>
>>> Since this change affects the way in which source check diagnostics 
>>> are generated, quite a lot of changes were required to adjust 
>>> expected output in golden files. Aside from those changes, I also 
>>> addressed the following issues:
>>>
>>> * JavaTokenizer was not using SOURCE_LEVEL flags for its 
>>> diagnostics, and it was not using the new diagnostic framework to 
>>> generate errors, which made the porting harder - so I've fixed that
>>> * CALog in Completeness analyzer required a new override for an 
>>> AbstractLog::error variant that was probably forgotten (this was 
>>> required otherwise CompletenessAnalyzerTest was failing after the 
>>> change above)
>>> * Log has a 'feature' so that diagnostics marked with the 
>>> SOURCE_LEVEL flag can be reported only once per source. But this 
>>> check was only looking at the 'head' of a diagnostic, ignoring all 
>>> arguments. Since now all source level check diagnostics share the 
>>> same head, I needed to tweak the logic to also compare arguments - 
>>> or else, one source level check error being reported would also 
>>> disable unrelated source level check diags.
>>>
>>> Then there were a bunch of mechanical translation, for instance 
>>> idioms such as:
>>>
>>> allowSimplifiedVarargs = source.allowSimplifiedVarargs();
>>>
>>> Were rewritten as:
>>>
>>> allowSimplifiedVarargs = source.allowed(Feature.SIMPLIFIED_VARARGS);
>>>
>>> Also, in JavacParser, this idiom:
>>>
>>> checkTypeAnnotations();
>>>
>>> has been replaced with
>>>
>>> checkSourceLevel(Feature.TYPE_ANNOTATIONS);
>>>
>>> Where checkSourceLevel is a new general routine that works on any 
>>> Feature.
>>>
>>> Finally, where possible, I also got rid of the various boolean flags 
>>> of the kind 'allowPrivateMethodInInterfaces' - if the class already 
>>> had a 'source' field, the boolean flag can be omitted, and the check 
>>> can be performed on the source object itself.
>>>
>>> Cheers
>>> Maurizio
>>>
>>>
>>>
>>
>