review request for 8010225 test in typeAnnotations/failures do not test TYPE_USE

[Steve Sides]

On 9/13/2013 10:42 AM, Alex Buckley wrote:
> - AnnotationVersion: The @summary is imprecise - the test is about the 
> receiver parameter 'this', not type annotations in general. In SE 6 
> and 7 and 8, the annotation type 'A' with no 
I'm not so sure I agree with that. Due to the 3 @compile lines, the test 
is about allowing type-annotations only with jdk8. The fact that he put 
the annotation on 'this' appears to me to be incidental.  It may be odd 
that he chose 'this' as opposed to maybe 'String' or some other more 
common usage, but it does seem to be just a usage to make the general 
point and elicit the diagnostic 
"compiler.err.type.annotations.not.supported.in.source: 1.7", and not to 
test that specific usage.

> @Target is applicable to formal parameter declarations, so @A in front 
> of a formal parameter would usually be a perfectly ordinary 
> declaration annotation. However, a formal parameter called 'this' is 
> not allowed in SE 6 or 7. In SE 8, 'this' is the receiver parameter 
> (not a formal parameter!) and is legal syntax - but annotation type 
> 'A' is not applicable to the type of the receiver parameter, so a 
> different error is due than in 6/7.
>
> For completeness, there should be an accompanying test in this 
> directory where 'A' has an explicit @Target(PARAMETER). 
> AnnotationVersion is a poor name; good names would be 
> AnnotatedReceiverParameter and AnnotatedReceiverParameterWithTarget.
>
> - Scopes: The @summary is inconsistent with the rest of the file. 
> Also, line 15, the @Target meta-annotation which is commented out, 
> should be deleted entirely, as the test relies on @Target _not_ being 
> present. A commented-out line looks like someone forgot to change 
> something.
Agreed. This test needs something or something less, or a little of 
both. "Scopes" seems a little grand for 1 check.  I wasn't sure about 
the comment. Maybe I was squinting and looking sideways to see it as 
making the point more explicit, but then it could also be seen as a left 
over.

-steve

>
> Alex
>
> On 9/13/2013 8:53 AM, Steve Sides wrote:
>> ..just pinging...
>> On 9/4/2013 12:40 PM, Steve Sides wrote:
>>> .after screwing up the webrev a little (it didn't scp like I thought
>>> it would), I created a new one:
>>> http://cr.openjdk.java.net/~ssides/8010225.2/
>>> with this changes:
>>>
>>> >test/tools/javac/annotations/typeAnnotations/failures/AnnotatedImport.java 
>>>
>>>
>>> >test/tools/javac/annotations/typeAnnotations/failures/AnnotatedPackage1.java 
>>>
>>>
>>> >test/tools/javac/annotations/typeAnnotations/failures/AnnotatedPackage2.java 
>>>
>>>
>>> I made the sorta unnecessary changes, since these are supposed to test
>>> test-annotations. Like you said, they're not wrong. :)
>>>
>>> >test/tools/javac/annotations/typeAnnotations/failures/AnnotationVersion.java 
>>>
>>>
>>> added positive test case of @compile AnnotationVersion.java
>>>
>>> >test/tools/javac/annotations/typeAnnotations/failures/IncompleteVararg.java 
>>>
>>>
>>> I agree, these are kind of bad syntax with or without any anntations,
>>> so I deleted the test.
>>>
>>> >test/tools/javac/annotations/typeAnnotations/failures/OldArray.java
>>> added out file and ref= to test file.
>>>
>>> >test/tools/javac/annotations/typeAnnotations/failures/Scopes.java
>>> I didn't mean to do more. Since I had them all loaded into an editor,
>>> as I clicked through them, I made it a 4 space indent.
>>> It's a superfluous but not incorrect change.
>>>
>>> >test/tools/javac/annotations/typeAnnotations/failures/StaticFields.java 
>>>
>>> corrected usage (compile with no annotations) and added other static
>>> contexts and updated .out file.
>>>
>>> >test/tools/javac/annotations/typeAnnotations/failures/StaticMethods.java 
>>>
>>> deleted in lieue of above changes.
>>>
>>> -steve
>>>
>>> On 8/30/2013 3:46 PM, Jonathan Gibbons wrote:
>>>> On 08/08/2013 05:48 PM, Steve Sides wrote:
>>>>> Some changes for JDK-8010225 : test in typeAnnotations/failures do
>>>>> not test TYPE_USE
>>>>> Bug: http://bugs.sun.com/view_bug.do?bug_id=8010225
>>>>> webrev: http://cr.openjdk.java.net/~ssides/8010225/
>>>>>
>>>>> This fix is for tl/langtools but since it's all type-annotations
>>>>> test I've posted it here.
>>>>>
>>>>> -steve
>>>>
>>>>
>>>> Here's the list of files:
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/AnnotatedImport.java 
>>>>
>>>>
>>>> OK.  Edit is sorta unnecessary, but not wrong.
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/AnnotatedPackage1.java 
>>>>
>>>>
>>>> OK.  Edit is sorta unnecessary, but not wrong.
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/AnnotatedPackage2.java 
>>>>
>>>>
>>>> OK.  Edit is sorta unnecessary, but not wrong.
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/AnnotationVersion.java 
>>>>
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/AnnotationVersion.out 
>>>>
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/AnnotationVersion7.out 
>>>>
>>>>
>>>> Test is only negative for 6, 7. Would be better to add positive test
>>>> without -source as well
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/BadCast.java
>>>> test/tools/javac/annotations/typeAnnotations/failures/BadCast.out
>>>> OK
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/CantAnnotateStaticClass.java 
>>>>
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/CantAnnotateStaticClass.out 
>>>>
>>>>
>>>> OK
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/IncompleteArray.java 
>>>>
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/IncompleteArray.out 
>>>>
>>>>
>>>> OK
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/IncompleteVararg.java 
>>>>
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/IncompleteVararg.out 
>>>>
>>>>
>>>> *** There's no varargs in this test, so this test is kinda weird
>>>> As it stands, it's sort of a random syntax error test.
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/IndexArray.java
>>>> test/tools/javac/annotations/typeAnnotations/failures/IndexArray.out
>>>> OK
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/OldArray.java
>>>> *** The test passes without changing the golden file. This is
>>>> unexpected.
>>>> The answer is because there is no /ref=...OldArray.out on line 29.
>>>> This should be fixed, and then, you'll need to create a golden file!
>>>> Also, remove the legal header and substitute /nodynamiccopyright/.
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/Scopes.java
>>>> *** You've just removed a space from the line, nothing more.
>>>> Did you mean to do more?
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/StaticFields.java 
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/StaticFields.out
>>>> *** Sorta OK, but not really.  The test runs the risk of  false 
>>>> positive
>>>> because f is not itself declared static.  Thus, quite apart from the
>>>> annotation, you are trying to do static access to a non-static field,
>>>> which is guaranteed to fail.
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/StaticMethods.java 
>>>>
>>>> test/tools/javac/annotations/typeAnnotations/failures/StaticMethods.out 
>>>>
>>>> This test is kinda pointless because the code fails even without the
>>>> annotation.
>>>> i.e. remove the annotation and the test still fails to compile.
>>>
>>