Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sqale support for 'other' rules #467

Closed
guwirth opened this issue Apr 16, 2015 · 12 comments · Fixed by #529
Closed

Sqale support for 'other' rules #467

guwirth opened this issue Apr 16, 2015 · 12 comments · Fixed by #529

Comments

@guwirth
Copy link
Collaborator

guwirth commented Apr 16, 2015

With new API it is no problem to set default values for Sqale/Technical Debt:
http://grepcode.com/file/repo1.maven.org/maven2/org.codehaus.sonar/sonar-plugin-api/4.5.2/org/sonar/api/server/rule/RulesDefinition.java#RulesDefinition.NewRule

[Public Method] setDebtRemediationFunction(DebtRemediationFunction) : RulesDefinition.NewRule
[Public Method] setDebtSubCharacteristic(String) : RulesDefinition.NewRule
[Public Method] setEffortToFixDescription(String) : RulesDefinition.NewRule

Question is only what is 'default'?

The current used RulesDefinitionXmlLoader does not support Technical Debt values:
http://javadocs.sonarsource.org/4.4/apidocs/org/sonar/api/server/rule/RulesDefinitionXmlLoader.html

<rules>
   <rule>
     <!-- required fields -->
     <key>the-rule-key</key>
     <name>The purpose of the rule</name>

     <!-- optional fields -->
     <description>
       <![CDATA[The description]]>
     </description>
     <internalKey>Checker/TreeWalker/LocalVariableName</internalKey>
     <severity>BLOCKER</severity>
     <cardinality>MULTIPLE</cardinality>
     <status>BETA</status>
     <param>
       <key>the-param-key</key>
       <tag>style</tag>
       <tag>security</tag>
       <description>
         <![CDATA[the param-description]]>
       </description>
       <defaultValue>42</defaultValue>
     </param>
     <param>
       <key>another-param</key>
     </param>

     <!-- deprecated fields -->
     <configKey>Checker/TreeWalker/LocalVariableName</configKey>
     <priority>BLOCKER</priority>
   </rule>
 </rules>

Idea could be to extend this file with Sqale values for the rules, so everything is together in one file?

<rule>
  ...
  <sqale>
    <chc>
      <key>PORTABILITY</key>
      <name>Portability</name>
        <chc>
          <key>COMPILER_RELATED_PORTABILITY</key>
          <name>Compiler related portability</name>
            <chc>
              <rule-repo>cppcheck</rule-repo>
              <rule-key>AssignmentAddressToInteger</rule-key>
              <prop>
                <key>remediationFunction</key>
                <txt>linear</txt>
              </prop>
              <prop>
                <key>remediationFactor</key>
                <val>5</val>
                <txt>mn</txt>
              </prop>
              <prop>
                <key>offset</key>
                <val>0.0</val>
                <txt>d</txt>
              </prop>
            ...
@jmecosta
Copy link
Member

The default is used to revert the rule to the original value, since those
can be modified when you create a new profile. If you query the rest api
you will get both default and current.

On Thu, Apr 16, 2015, 10:15 Günter Wirth [email protected] wrote:

With new API it is no problem to set default values for Sqale/Technical
Debt:

http://grepcode.com/file/repo1.maven.org/maven2/org.codehaus.sonar/sonar-plugin-api/4.5.2/org/sonar/api/server/rule/RulesDefinition.java#RulesDefinition.NewRule

[Public Method] setDebtRemediationFunction(DebtRemediationFunction) : RulesDefinition.NewRule
[Public Method] setDebtSubCharacteristic(String) : RulesDefinition.NewRule
[Public Method] setEffortToFixDescription(String) : RulesDefinition.NewRule

Question is only what is 'default'?

The current used RulesDefinitionXmlLoader does not support Technical Debt
values:

http://javadocs.sonarsource.org/4.4/apidocs/org/sonar/api/server/rule/RulesDefinitionXmlLoader.html

the-rule-key The purpose of the rule
 <!-- optional fields -->
 <description>
   <![CDATA[The description]]>
 </description>
 <internalKey>Checker/TreeWalker/LocalVariableName</internalKey>
 <severity>BLOCKER</severity>
 <cardinality>MULTIPLE</cardinality>
 <status>BETA</status>
 <param>
   <key>the-param-key</key>
   <tag>style</tag>
   <tag>security</tag>
   <description>
     <![CDATA[the param-description]]>
   </description>
   <defaultValue>42</defaultValue>
 </param>
 <param>
   <key>another-param</key>
 </param>

 <!-- deprecated fields -->
 <configKey>Checker/TreeWalker/LocalVariableName</configKey>
 <priority>BLOCKER</priority>

Idea could be to extend this file with Sqale values for the rules, so
everything is together in one file?

... PORTABILITY Portability COMPILER_RELATED_PORTABILITY Compiler related portability cppcheck AssignmentAddressToInteger remediationFunction linear remediationFactor 5 mn offset 0.0 d ...


Reply to this email directly or view it on GitHub
#467.

@guwirth
Copy link
Collaborator Author

guwirth commented Apr 16, 2015

Don't understand this, calling e.g. http://localhost:9000/api/rules/show?key=other:PORTING.BSWAP.MACRO

I get:

{ 
   "rule":{  
      "key":"other:PORTING.BSWAP.MACRO",
      "repo":"other",
      "name":"PORTING.BSWAP.MACRO:Use of a custom byte-swap macro without endianness check",
      "createdAt":"2015-04-09T12:08:21+0200",
      "severity":"MAJOR",
      "status":"READY",
      "internalKey":"PORTING.BSWAP.MACRO",
      "isTemplate":false,
      "tags":[ 
      ],
      "sysTags":[ 
      ],
      "lang":"c++",
      "langName":"c++",
      "htmlDesc":"Use of a custom byte-swap macro without endianness check",
      "debtOverloaded":false,
      "params":[ 
      ]
   }
}

Calling http://localhost:9000/api/rules/show?key=cxx:HardcodedAccount

I get

{  
   "rule":{  
      "key":"cxx:HardcodedAccount",
      "repo":"cxx",
      "name":"Do not hard code sensitive data in programs",
      "createdAt":"2015-04-09T11:59:39+0200",
      "severity":"BLOCKER",
      "status":"READY",
      "internalKey":"HardcodedAccount",
      "isTemplate":false,
      "tags":[  
      ],
      "sysTags":[  
         "cert",
         "security"
      ],
      "lang":"c++",
      "langName":"c++",
      "htmlDesc":"<p>...</p>",
      "defaultDebtChar":"CHANGEABILITY",
      "defaultDebtSubChar":"ARCHITECTURE_CHANGEABILITY",
      "debtChar":"CHANGEABILITY",
      "debtSubChar":"ARCHITECTURE_CHANGEABILITY",
      "debtCharName":"Changeability",
      "debtSubCharName":"Architecture",
      "defaultDebtRemFnType":"CONSTANT_ISSUE",
      "defaultDebtRemFnOffset":"30min",
      "debtOverloaded":false,
      "debtRemFnType":"CONSTANT_ISSUE",
      "debtRemFnOffset":"30min",
      "params":[  
         {  
            "key":"regularExpression",
            "htmlDesc":"literal regular expression rule",
            "type":"STRING",
            "defaultValue":"\\bDSN\\b.*=.*;\\b(UID|PWD)\\b=.*;"
         }
      ]
   }
}

In second case there are

      "defaultDebtChar":"CHANGEABILITY",
      "defaultDebtSubChar":"ARCHITECTURE_CHANGEABILITY",
      "debtChar":"CHANGEABILITY",
      "debtSubChar":"ARCHITECTURE_CHANGEABILITY",
      "debtCharName":"Changeability",
      "debtSubCharName":"Architecture",
      "defaultDebtRemFnType":"CONSTANT_ISSUE",
      "defaultDebtRemFnOffset":"30min",
      "debtOverloaded":false,
      "debtRemFnType":"CONSTANT_ISSUE",
      "debtRemFnOffset":"30min",

So my idea was to add this also for 'other'.

  • Step 1: a default value
  • Step 2: add a possibility to define them also in the XML file

@jmecosta
Copy link
Member

Sure if platform supports its fine, I usually use this
https://github.com/jmecsoftware/QualityProfileEditorPlugin since i can
adjust those ones on the fly.

On Thu, Apr 16, 2015, 12:38 Günter Wirth [email protected] wrote:

Don't understand this, calling e.g.
http://localhost:9000/api/rules/show?key=other:PORTING.BSWAP.MACRO

I get:

{
"rule":{
"key":"other:PORTING.BSWAP.MACRO",
"repo":"other",
"name":"PORTING.BSWAP.MACRO:Use of a custom byte-swap macro without endianness check",
"createdAt":"2015-04-09T12:08:21+0200",
"severity":"MAJOR",
"status":"READY",
"internalKey":"PORTING.BSWAP.MACRO",
"isTemplate":false,
"tags":[
],
"sysTags":[
],
"lang":"c++",
"langName":"c++",
"htmlDesc":"Use of a custom byte-swap macro without endianness check",
"debtOverloaded":false,
"params":[
]
}
}

Calling http://localhost:9000/api/rules/show?key=cxx:HardcodedAccount

I get

{
"rule":{
"key":"cxx:HardcodedAccount",
"repo":"cxx",
"name":"Do not hard code sensitive data in programs",
"createdAt":"2015-04-09T11:59:39+0200",
"severity":"BLOCKER",
"status":"READY",
"internalKey":"HardcodedAccount",
"isTemplate":false,
"tags":[
],
"sysTags":[
"cert",
"security"
],
"lang":"c++",
"langName":"c++",
"htmlDesc":"

...

",
"defaultDebtChar":"CHANGEABILITY",
"defaultDebtSubChar":"ARCHITECTURE_CHANGEABILITY",
"debtChar":"CHANGEABILITY",
"debtSubChar":"ARCHITECTURE_CHANGEABILITY",
"debtCharName":"Changeability",
"debtSubCharName":"Architecture",
"defaultDebtRemFnType":"CONSTANT_ISSUE",
"defaultDebtRemFnOffset":"30min",
"debtOverloaded":false,
"debtRemFnType":"CONSTANT_ISSUE",
"debtRemFnOffset":"30min",
"params":[
{
"key":"regularExpression",
"htmlDesc":"literal regular expression rule",
"type":"STRING",
"defaultValue":"\bDSN\b.=.;\b(UID|PWD)\b=.*;"
}
]
}
}

In second case there are

  "defaultDebtChar":"CHANGEABILITY",
  "defaultDebtSubChar":"ARCHITECTURE_CHANGEABILITY",
  "debtChar":"CHANGEABILITY",
  "debtSubChar":"ARCHITECTURE_CHANGEABILITY",
  "debtCharName":"Changeability",
  "debtSubCharName":"Architecture",
  "defaultDebtRemFnType":"CONSTANT_ISSUE",
  "defaultDebtRemFnOffset":"30min",
  "debtOverloaded":false,
  "debtRemFnType":"CONSTANT_ISSUE",
  "debtRemFnOffset":"30min",

So my idea was to add this also for 'other'.

  • Step 1: a default value
  • Step 2: add a possibility to define them also in the XML file


Reply to this email directly or view it on GitHub
#467 (comment).

@guwirth
Copy link
Collaborator Author

guwirth commented Apr 16, 2015

I think your tool is very good to modify the settings but for an initial setup I would prefer to have all in one file and inside of the SQ configuration.

@jmecosta
Copy link
Member

Sure I can see the point. Perhaps augment the current format with the sqale
definitions

On Thu, Apr 16, 2015, 16:40 Günter Wirth [email protected] wrote:

I think your tool is very good to modify the settings but for an initial
setup I would prefer to have all in one file and inside of the SQ
configuration.


Reply to this email directly or view it on GitHub
#467 (comment).

@guwirth
Copy link
Collaborator Author

guwirth commented Apr 16, 2015

Do you know if there is a helper class to read the sqale xml (<sqale><chc>...)?

@jmecosta
Copy link
Member

not aware of anything... perhaps the sonar core contains something

On Thu, 16 Apr 2015 at 18:49 Günter Wirth [email protected] wrote:

Do you know if there is a helper class to read the sqale xml (
...)?


Reply to this email directly or view it on GitHub
#467 (comment).

@guwirth
Copy link
Collaborator Author

guwirth commented Apr 17, 2015

See #469 for first demo to set a 'default' sqale value.

@guwirth guwirth modified the milestone: M 0.9.4 Apr 29, 2015
@guwirth
Copy link
Collaborator Author

guwirth commented May 29, 2015

@jmecosta
Copy link
Member

Yep that will work. Good that they introduce this method. I suggest the
sqale XML also to be added via the web ui and this method will load the
definitions

On Fri, May 29, 2015, 22:25 Günter Wirth [email protected] wrote:

This could be a solution:
http://grepcode.com/file/repo1.maven.org/maven2/org.codehaus.sonar.sslr-squid-bridge/sslr-squid-bridge/2.5/org/sonar/squidbridge/rules/SqaleXmlLoader.java.
Load in the same way as rule file.


Reply to this email directly or view it on GitHub
#467 (comment).

@guwirth
Copy link
Collaborator Author

guwirth commented May 30, 2015

Still not sure what is the better choice here:

  • have own xml file for rules and sqale values
  • or have two separated files?

@jmecosta
Copy link
Member

Perhaps the existing XML could be augmented to include the chars. Still
keeping the current format intact. We might need to create a script to
validate the characteristics since the model is very specific and it's easy
to make mistakes

On Sat, May 30, 2015, 13:04 Günter Wirth [email protected] wrote:

Still not sure what is the better choice here:

  • have own xml file for rules and sqale values
  • or have two separated files?


Reply to this email directly or view it on GitHub
#467 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants