Revise metadata of some rules #351

Merged
merged 3 commits into from Sep 7, 2016

Projects

None yet

3 participants

@ryuichis
Member
ryuichis commented Jul 12, 2016 edited

Refining the metadata of rules is an on-going process. We recently hear some confusions among the rule name, identifier, and attribute name to be used. By following a sequence of rule changes, document enhancement, and automations, we revise the metadata of some of the rules.

Improvements and suggestions are welcomed.

Identifier changes incorporated in this pull request:

  • "MustOverrideHashWithIsEqual" to "MissingHashMethod"
  • "MustCallSuper" to "MissingCallToBaseMethod"
  • "VerifyProhibitedCall" to "CallingProhibitedMethod"
  • "VerifyProtectedMethod" to "CallingProtectedMethod"
  • "SubclassMustImplement" to "MissingAbstractMethodImplementation"
  • "BaseClassDestructorShouldBeVirtualOrProtected" to "ProblematicBaseClassDestructor"
  • "CoveredSwitchStatementsDontNeedDefault" to "UnnecessaryDefaultStatement"
  • "DefaultLabelNotLastInSwitchStatement" to "MisplacedDefaultLabel"
  • "IvarAssignmentOutsideAccessorsOrInit" to "AssignIvarOutsideAccessors"
  • "UseEarlyExitsAndContinue" to "PreferEarlyExit"
  • "SwitchStatementsShouldHaveDefault" to "MissingDefaultStatement"
  • "ReplaceWithBoxedExpression" to "UseBoxedExpression"
  • "ReplaceWithContainerLiteral" to "UseContainerLiteral"
  • "ReplaceWithNumberLiteral" to "UseNumberLiteral"
  • "ReplaceWithObjectSubscripting" to "UseObjectSubscripting"

Rule name changes incorporated in this pull request:

  • "must override hash with isEqual" to "missing hash method"
  • "must call super" to "missing call to base method"
  • "verify prohibited call" to "calling prohibited method"
  • "verify protected method" to "calling protected method"
  • "subclass must implement" to "missing abstract method implementation"
  • "covered switch statements dont need default" to "unnecessary default statement in covered switch statement"
  • "default label not last in switch statement" to "ill-placed default label in switch statement"
  • "use early exits and continue" to "prefer early exits and continue"
  • "switch statements should have default" to "missing default in switch statements"
  • "replace with boxed expression" to "use boxed expression"
  • "replace with container literal" to "use container literal"
  • "replace with number literal" to "use number literal"
  • "replace with object subscripting" to "use object subscripting"

Attribute name changes incorporated in this pull request:

  • "must call super" to "base method"
  • "prohibited call" to "prohibited method"
  • "subclass must implement" to "abstract method"

Coming document changes:

We will expose the details of rules more explicitly in our documentation as well:

rule_doc_illustration

@ryuichis ryuichis Some rule name changes
c4fb4e3
@UnrealQuester
Member

👍
I really like these new changes. This should clear up some confusion about the rule names and identifiers.

"must call super" to "missing calling base method"

I'm not a native english speaker, but I think "missing call to base method" is correct here.

"DefaultLabelNotLastInSwitchStatement" to "IllplacedDefaultLabel"

The uppercase 'I' followed by two lowercase 'l's make the name somewhat hard to read, but I cannot think of a good replacement.

@ryuichis
Member

@UnrealQuester I am not native English speaker either, but I like both of your suggestions! I will think about a better name for Illplaced, and work on this later.

@ryuichis ryuichis Rename MissingCallingBaseMethod to MissingCallToBaseMethod per @Unrea…
…lQuester suggestion
c428333
@ryuichis
Member
ryuichis commented Sep 5, 2016

@UnrealQuester I have changed the MissingCallToBaseMethod per your suggestion, and it is hard for me to find a better alternative for illplaced.

I did some diggings from dictionary:

  • misplaced: incorrectly positioned.
  • displaced: replaced.
  • unplaced: not having or assigned to a specific place; not appropriate or correct in the circumstances.

Based on my understanding of these terms, they are all somehow off from the meaning of ill-placed.

By looking at the definition of ill-, I was wondering if we can say bad placed as in this Quora post. So basically BadPlacedDefaultLabel. What do you think?

@UnrealQuester
Member

@ryuichis My vote goes to MisplacedDefaultLabel.

@ryuichis ryuichis Rename IllplacedDefaultLabel to MisplacedDefaultLabel
cab3735
@coveralls

Coverage Status

Coverage decreased (-1.5%) to 82.947% when pulling cab3735 on ryuichis:rule-retrospective into 0cafd45 on oclint:master.

@ryuichis
Member
ryuichis commented Sep 7, 2016

Alright, changes appended. So I am going to merge this one, and if we have further suggestions, I'll work on them on separate pull requests. Again thanks @UnrealQuester for providing the review and suggestions.

@ryuichis ryuichis merged commit fa6a904 into oclint:master Sep 7, 2016

1 of 2 checks passed

coverage/coveralls Coverage decreased (-1.5%) to 82.947%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ryuichis ryuichis deleted the ryuichis:rule-retrospective branch Sep 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment