[jbosstools-issues] [JBoss JIRA] (JBIDE-10187) Add support for a @SuppressWarnings

[Viacheslav Kabanovich (Issue Comment Edited) (JIRA)]

    [ https://issues.jboss.org/browse/JBIDE-10187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12651090#comment-12651090 ] 

Viacheslav Kabanovich edited comment on JBIDE-10187 at 12/14/11 6:22 PM:
-------------------------------------------------------------------------

1. If there is already SuppressWarnings annotation with array value
{code}
@SuppressWarnings({"xx"})
{code}
then the result of quick fix is
{code}
@SuppressWarnings({"[Ljava.lang.Object;@f2cff4", "org.jboss.tools.cdi.seam.solder.core.validator.problem.genericConfigurationTypeIsGenericBean"})
{code}
Method AddSuppressWarningsMarkerResolution.updateAnnotation should check if the current value is an array.

2. If there is already SuppressWarnings annotation with qualified/simple name value
{code}
@SuppressWarnings(A.MY_NAME)
{code}
where MY_NAME is String constant in interface A,
then the result of quick fix is
{code}
@SuppressWarnings({"A.MY_NAME", "org.jboss.tools.cdi.seam.solder.core.validator.problem.genericConfigurationTypeIsGenericBean"})
{code}
Method AddSuppressWarningsMarkerResolution.updateAnnotation should check if the current value (or element of array) is a qualified or simple name and not include it into quotes.

3. Just IMHO - it may be better to name button 'Project' than type project name in it, that name can be quite long (see ButtonNameBorrowedFromProject.png).

4. That one may be hard to fix: after quick fix, editor is scrolled to the beginning. If Undo/Redo commands are used, the entire file content gets selected, so that is not obvious what change was made. Compare to what Java quick fixes do. I guess that we should try to use their approach, maybe create and apply CompilationUnitChange instead writing to buffer. We have always used buffer before, and the same issue may be found in all those cases, but just now I realized that it may become an annoyance for the user.

5. When new SuppressWarning annotation is created for
{code}
   /*@Inject has indent*/
   @Inject
   Object ooo;
{code}
annotation @Inject loses its indent:
{code}
   /*@SuppressWarnings has indent, @Inject does not*/
   @SuppressWarnings("org.jboss.tools.cdi.core.validator.problem.unsatisfiedInjectionPoints")
@Inject
   Object ooo;
{code}

6. When the quick fix is run on a method parameter, annotation is added to method, that is fine, but it could be added to parameter itself as well; I have checked - our validation takes it into account in both places. Java in such a case suggests two quick fixes, try it for
{code}
   public void a(Class ooo) {};
{code}
There are two options:
a) Add @SuppressWarnings 'rawtypes' to 'ooo'
b) Add @SuppressWarnings 'rawtypes' to 'a()'

I think we should follow Java's algorithm as close as possible.

7. As a suggestion, may it be good to set quick fix label to e.g. "Add @SuppressWarnings 'unsatisfiedInjectionPoints' to ooo" instead of "Add @SuppressWarnings to ooo"? It is not good to put our long qualified preference name, but what goes after the last dot is no less user friendly than 'rawtypes'. In the case of multiple markers, if there are other quick fixes to suggest "Add @SuppressWarnings ...", then they become confusing without specifying what exact warning is going to be suppressed (see ManySuggestionsToAddSuppressWarnings.png).

8. We suggest that quick fix, when the level is set to 'Error'. Of course, the SuppressWarnings annotation does not help in that case.

                
      was (Author: scabanovich):
    1. If there is already SuppressWarnings annotation with array value
{code}
@SuppressWarnings({"xx"})
{code}
then the result of quick fix is
{code}
@SuppressWarnings({"[Ljava.lang.Object;@f2cff4", "org.jboss.tools.cdi.seam.solder.core.validator.problem.genericConfigurationTypeIsGenericBean"})
{code}
Method AddSuppressWarningsMarkerResolution.updateAnnotation should check if the current value is an array.

2. If there is already SuppressWarnings annotation with qualified/simple name value
{code}
@SuppressWarnings(A.MY_NAME)
{code}
where MY_NAME is String constant in interface A,
then the result of quick fix is
{code}
@SuppressWarnings({"A.MY_NAME", "org.jboss.tools.cdi.seam.solder.core.validator.problem.genericConfigurationTypeIsGenericBean"})
{code}
Method AddSuppressWarningsMarkerResolution.updateAnnotation should check if the current value (or element of array) is a qualified or simple name and not include it into quotes.

3. Just IMHO - it may be better to name button 'Project' than type project name in it, that name can be quite long (see ButtonNameBorrowedFromProject.png).

4. That one may be hard to fix: after quick fix, editor is scrolled to the beginning. If Undo/Redo commands are used, the entire file content gets selected, so that is not obvious what change was made. Compare to what Java quick fixes do. I guess that we should try to use there approach, maybe create and apply CompilationUnitChange instead writing to buffer. We have always used buffer before, and the same issue may be found in all those cases, but just now I realized that it may become an annoyance for the user.

5. When new SuppressWarning annotation is created for
{code}
   @Inject
   Object ooo;
{code}
annotation @Inject loses its indent:
{code}
   @SuppressWarnings("org.jboss.tools.cdi.core.validator.problem.unsatisfiedInjectionPoints")
@Inject
   Object ooo;
{code}

6. When the quick fix is run on a method parameter, annotation is added to method, that is fine, but it could be added to parameter itself as well; I have checked - our validation takes it into account in both places. Java in such a case suggests two quick fixes, try it for
{code}
   public void a(Class ooo) {};
{code}
There are two options:
a) Add @SuppressWarnings 'rawtypes' to 'ooo'
b) Add @SuppressWarnings 'rawtypes' to 'a()'

I think we should follow Java's algorithm as close as possible.

7. As a suggestion, may it be good to set quick fix label to e.g. "Add @SuppressWarnings 'unsatisfiedInjectionPoints' to ooo" instead of "Add @SuppressWarnings to ooo"? It is not good to put our long qualified preference name, but what goes after the last dot is no less user friendly than 'rawtypes'. In the case of multiple markers, if there are other quick fixes to suggest "Add @SuppressWarnings ...", then they become confusing without specifying what exact warning is going to be suppressed (see ManySuggestionsToAddSuppressWarnings.png).

8. We suggest that quick fix, when the level is set to 'Error'. Of course, the SuppressWarnings annotation does not help in that case.

                  
> Add support for a @SuppressWarnings
> -----------------------------------
>
>                 Key: JBIDE-10187
>                 URL: https://issues.jboss.org/browse/JBIDE-10187
>             Project: Tools (JBoss Tools)
>          Issue Type: Feature Request
>          Components: common/jst/core
>    Affects Versions: 3.3.0.M4
>            Reporter: Cody Lerum
>            Assignee: Viacheslav Kabanovich
>             Fix For: 3.3.0.Beta1
>
>         Attachments: Add_ at SuppressWarnings.png, Add_ at SuppressWarnings2.png, Add_ at SuppressWarnings2.png, Add_ at SuppressWarnings3.png, Add_ at SuppressWarnings3.png, ButtonNameBorrowedFromProject.png, ManySuggestionsToAddSuppressWarnings.png
>
>
> Would be nice to place a 
> @SuppressWarning("ambigiousinjection") or similar on an injection point that tooling can't figure out.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira