Note draft / work in progress
Rule violations:
- that are critical or blocker generally indicate bugs and should be fixed as soon possible.
- that are major may point to areas where issues can occur and care should be taken. You may want to fix these.
- that are minor can be generally ignored but you may want to comply with them when writing new code.
Fixed here means fixed if there is a bug or marked as won't fix or false positive in the Sonar Cube web interface if it isn't bug.
Note rules that are not commonly encountered, don't occur in the current code base or are minor are not mentioned here.
The resulting rule changes will result in a more relaxed set of rules that match the current development code style of the FlexJS framework and that make the Sonar Cube reports more useful.
For a list of all rules, code example and rule references see here.
New Rules
These rules (currently disabled) should be added:
- Copyright and license headers should be defined (blocker)
- Public classes, methods, properties and metadata should be documented with ASDoc (major)
- Method visibility should be explicitly declared (minor)
Critical or Blocking Rules
Any class extending the Event class should override Event.clone()
This is often a bug as without the clone (or similar) method events will not bubble correctly. A false positive may be the case when bubbles is not passed into the constructor but that may also be an omission.
Other rules while probably require no discussion as they generally indicate a serious issue or outdated coding style include:
- Constructors should not dispatch events
- "with" statements should not be used
- Cases in a "switch" should not have the same condition
- Statements, operators and keywords specific to ActionScript 2 should not be used
- Switch cases should end with an unconditional "break" statement
Move Rules
These critical rues can be moved to major.
At some point we may want to consider moving the trace rule back and replacing trace statements with logging calls. (This has been discussed on the dev list.)
Major Rules
Current major rules we should keep. Most of these are simple and safe to fix in some cases provide performance and or code size improvements.
- "for" loop stop conditions should be invariant
As to most rules their exceptions, but the most common issue here is checking the length of an array every time around the loop.
- Collapsible "if" statements should be merged
- Constructor bodies should be as lightweight as possible
Code in AS constructors is not JITed. This only effect AS code not the generated JS code. IS performance on AS side a consideration? - Event names should not be hardcoded in event listeners
This has been discussed on the mailing list. - Event types should be defined in metadata tags
- Local variables should not shadow class fields
- Objects should not be instantiated inside a loop
Sometime of course you do want to but in some cases this is an issue and can improve performance. - Unused local variables should be removed
- Unused private fields should be removed
- Unused private function should be removed
- Useless "if(true) {...}" and "if(false){...}" blocks should be removed
Move Some Major Rules to Minor
List of current major rules to moved to minor. These currently add a lot of noise or would require a fair amount of effort to comply with or may come with the risk of adding bugs. Moving them to minor would make it easier to see other major issues listed above.
- "===" and "!==" should be used instead of "==" and "!="
- "switch case" clauses should not have too many lines
- "switch" statements should end with a "default" clause
- Classes should not be too complex
- Classes should not have too many methods
- Control flow statements "if", "for", "while" and "switch" should not be nested too deeply
- Functions should not be too complex
- Functions should not contain too many return statements
- Functions should not have too many parameters
- Methods should not be empty
- Nested blocks of code should not be left empty
- Sections of code should not be "commented out"
- The element type of an array field should be specified
- The special "star" type should not be used
- Variables of the "Object" type should not be used
- Unused function parameters should be removed ?????
Move from Minor to Major
Missing semicolons may introduce errors when optimising and minimising JS code so the missing semicolons rule will move for minor to major. This are possibly other ways to get around this issue but the change is easy and the risk of introducing bugs very low.