SonarQube Plugins
  1. SonarQube Plugins
  2. SONARPLUGINS-2081

Conditional Comments should not be flagged by "Avoid Html Comment" and "Comments should not include code"

    Details

    • Number of attachments :
      0

      Description

      Since conditional comments (http://en.wikipedia.org/wiki/Conditional_comment) have a semantical meaning, they are also used within JSPs. Those kind of comments should be excluded from the "Avoid Html Comment" rule. For example this code gives false positives for all those conditional comments if used inside a JSP:

      <!--[if lt IE 7]> <html class="no-js ie6 oldie"> <![endif]-->
      <!--[if IE 7]>    <html class="no-js ie7 oldie"> <![endif]-->
      <!--[if IE 8]>    <html class="no-js ie8 oldie"> <![endif]-->
      <!--[if gt IE 8]><!--> <html class="no-js"> <!--<![endif]-->
      </html>
      

        Issue Links

          Activity

          Hide
          Dinesh Bolkensteyn added a comment -

          You're right, will try to fix.

          Show
          Dinesh Bolkensteyn added a comment - You're right, will try to fix.
          Hide
          Dinesh Bolkensteyn added a comment -

          Done, lost 14 issues on the ruling IT thanks to this - all were indeed false positives

          Show
          Dinesh Bolkensteyn added a comment - Done, lost 14 issues on the ruling IT thanks to this - all were indeed false positives
          Hide
          Freddy Mallet added a comment -

          Works fine for the "Comments should not include code" coding rule but not for the "Avoid Html comment" coding rule.

          Show
          Freddy Mallet added a comment - Works fine for the "Comments should not include code" coding rule but not for the "Avoid Html comment" coding rule.
          Hide
          Dinesh Bolkensteyn added a comment -

          Done

          Show
          Dinesh Bolkensteyn added a comment - Done
          Hide
          Dinesh Bolkensteyn added a comment -

          16 fewer issues on the ruling

          Show
          Dinesh Bolkensteyn added a comment - 16 fewer issues on the ruling
          Hide
          Freddy Mallet added a comment -

          Almost works Dinesh but all HTML comments located AFTER those conditional comments are also ignored. Here is an example:

          <!-- This is a real comment -->
          
          <!--[if lt IE 7]> <html class="no-js ie6 oldie"> <![endif]-->
          <!--[if IE 7]>    <html class="no-js ie7 oldie"> <![endif]-->
          <!--[if IE 8]>    <html class="no-js ie8 oldie"> <![endif]-->
          <!--[if gt IE 8]><!--> <html class="no-js"> <!--<![endif]-->
          
          <!-- This is a real comment -->
          
          Show
          Freddy Mallet added a comment - Almost works Dinesh but all HTML comments located AFTER those conditional comments are also ignored. Here is an example: <!-- This is a real comment --> <!--[ if lt IE 7]> <html class= "no-js ie6 oldie" > <![endif]--> <!--[ if IE 7]> <html class= "no-js ie7 oldie" > <![endif]--> <!--[ if IE 8]> <html class= "no-js ie8 oldie" > <![endif]--> <!--[ if gt IE 8]><!--> <html class= "no-js" > <!--<![endif]--> <!-- This is a real comment -->
          Hide
          Dinesh Bolkensteyn added a comment - - edited

          Indeed this seems to relate to a problem in the lexer.

          I created another ticket to cover that properly: SONARPLUGINS-3049

          Show
          Dinesh Bolkensteyn added a comment - - edited Indeed this seems to relate to a problem in the lexer. I created another ticket to cover that properly: SONARPLUGINS-3049
          Hide
          Dinesh Bolkensteyn added a comment -

          (there was no change in the code)

          Show
          Dinesh Bolkensteyn added a comment - (there was no change in the code)
          Hide
          Freddy Mallet added a comment -

          OK

          Show
          Freddy Mallet added a comment - OK

            People

            • Assignee:
              Dinesh Bolkensteyn
              Reporter:
              Konrad Windszus
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: