Upgrade to Pro — share decks privately, control downloads, hide ads and more …

When QualityAssistant Meets Pharo [Enforced Code Critiques Motivate More Valuable Rules] (iwst2016)

When QualityAssistant Meets Pharo [Enforced Code Critiques Motivate More Valuable Rules] (iwst2016)

Static analysis tools can aid in software quality assessment, but are rarely used by software developers. Poor usage of quality analysis tools not only means missed opportunities for the quality of software systems, but also results in little feedback, which in turn slows the improvements of the quality rules themselves.

We introduced a set of intrusive quality plugins and integrated them into the Pharo IDE. This not only triggered a feedback loop that led to improvements of the existing rules, but also encouraged removal of some rules and integration of new ones. Our analysis of changes to the rules suggests that precise rules capturing a domain-specific logic are more valuable than general ones.

Yuriy Tymchuk

August 23, 2016
Tweet

More Decks by Yuriy Tymchuk

Other Decks in Programming

Transcript

  1. @yuriy_tymchuk
    When QualityAssistant Meets Pharo
    Enforced Code Critiques Motivate
    More Valuable Rules
    , Mohammad Ghafari, Oscar Nierstrasz
    Software Composition Group @ University of Bern

    View Slide

  2. The Concept

    View Slide

  3. The Concept
    Rule

    View Slide

  4. The Concept
    Rule

    View Slide

  5. The Concept
    Rule Critique
    produces

    View Slide

  6. The Concept
    Rule Critique
    produces

    View Slide

  7. The Concept
    Rule Critique
    [|]
    Code
    produces
    improves

    View Slide

  8. The Concept
    Rule Critique
    [|]
    Code
    produces
    improves
    (feedback)
    improves

    View Slide

  9. The Concept
    Rule Critique
    [|]
    Code
    produces
    improves
    (feedback)
    improves

    View Slide

  10. The Concept
    Rule Critique
    [|]
    Code
    produces
    improves

    View Slide

  11. The Concept
    Rule Critique
    [|]
    Code
    produces
    improves

    View Slide

  12. The Existing CriticBrowser

    View Slide

  13. Newly Introduced QualityAssistant

    View Slide

  14. SmallLint Issues per Milestone
    0
    10
    20
    30
    40
    50
    Pharo 3 Pharo 4 Pharo 5 Pharo 6*

    View Slide

  15. Changes in Rules

    View Slide

  16. Changes in Rules
    Fixed

    View Slide

  17. Changes in Rules
    Fixed Removed

    View Slide

  18. Added
    Changes in Rules
    Fixed Removed

    View Slide

  19. Fixed

    View Slide

  20. Fixed
    (Collection>>#add:) protocol #adding

    View Slide

  21. Fixed
    (Collection>>#add:) protocol
    (ArrayedCollection>>#add:) protocol #adding
    #adding

    View Slide

  22. Fixed
    (Collection>>#add:) protocol
    (ArrayedCollection>>#add:) protocol #adding
    #accessing

    View Slide

  23. Fixed
    (Collection>>#add:) protocol
    (ArrayedCollection>>#add:) protocol #adding
    #accessing

    View Slide

  24. Fixed
    (Collection>>#add:) protocol
    #'as yet unclassified'
    (ArrayedCollection>>#add:) protocol #adding

    View Slide

  25. Fixed
    (Collection>>#add:) protocol
    #'as yet unclassified'
    (ArrayedCollection>>#add:) protocol #adding
    Trivial Bug

    View Slide

  26. Fixed

    View Slide

  27. Fixed
    RBModi!esCollectionRule

    View Slide

  28. Fixed
    RBModi!esCollectionRule
    Horrible Bug

    View Slide

  29. Removed

    View Slide

  30. Removed
    Probably missing ‘; yourself’
    Non-blocks in special messages
    References an abstract class

    View Slide

  31. Removed
    Probably missing ‘; yourself’
    Non-blocks in special messages
    References an abstract class
    size = 1 ifTrue: ’:’ ifFalse: ’s:’

    View Slide

  32. Removed
    Probably missing ‘; yourself’
    Non-blocks in special messages
    References an abstract class
    String new
    size = 1 ifTrue: ’:’ ifFalse: ’s:’

    View Slide

  33. Removed
    Probably missing ‘; yourself’
    Non-blocks in special messages
    References an abstract class
    String new
    size = 1 ifTrue: ’:’ ifFalse: ’s:’
    Educational

    View Slide

  34. Added

    View Slide

  35. Added
    assert: a = b
    ifNotNilDo: ifNotNil:
    Smalltalk at: Smalltalk globals at:
    assert: a equals: b

    View Slide

  36. Added
    assert: a = b
    ifNotNilDo: ifNotNil:
    Smalltalk at: Smalltalk globals at:
    assert: a equals: b
    Migration

    View Slide

  37. Added
    assert: a = b
    ifNotNilDo: ifNotNil:
    Smalltalk at: Smalltalk globals at:
    assert: a equals: b
    Migration
    Recipe: use rewrite rules

    View Slide

  38. Added

    View Slide

  39. Added
    BoxedFloat64
    reference to

    View Slide

  40. Added
    BoxedFloat64
    reference to
    Private Access

    View Slide

  41. Added
    BoxedFloat64
    reference to
    Recipe: annotate/maintain a collection
    of system classes/methods
    Private Access

    View Slide

  42. Added

    View Slide

  43. Added
    b := RTMondrian new.
    b edges connectFrom: #superclass.
    b nodes: RTShape withAllSubclasses.

    View Slide

  44. Added
    b := RTMondrian new.
    b edges connectFrom: #superclass.
    b nodes: RTShape withAllSubclasses.
    b := RTMondrian new.
    b edges connectFrom: #superclass.
    b nodes: RTShape withAllSubclasses.

    View Slide

  45. Added
    b := RTMondrian new.
    b edges connectFrom: #superclass.
    b nodes: RTShape withAllSubclasses.
    Invocation order
    b := RTMondrian new.
    b edges connectFrom: #superclass.
    b nodes: RTShape withAllSubclasses.

    View Slide

  46. Added
    b := RTMondrian new.
    b edges connectFrom: #superclass.
    b nodes: RTShape withAllSubclasses.
    Recipe:
    initialize
    super initialize.
    self add: #edges requiresPreSend: #nodes:.
    Invocation Order
    b := RTMondrian new.
    b edges connectFrom: #superclass.
    b nodes: RTShape withAllSubclasses.

    View Slide

  47. Added

    View Slide

  48. Added
    ReAbstractRule class
    >> #checksPackage;
    >> #checksClass;
    >> #checksMethod;
    >> #checksNode

    View Slide

  49. Added
    ReAbstractRule class
    >> #checksPackage;
    >> #checksClass;
    >> #checksMethod;
    >> #checksNode
    true?

    View Slide

  50. Added
    ReAbstractRule class
    >> #checksPackage;
    >> #checksClass;
    >> #checksMethod;
    >> #checksNode
    true?
    Class Structure

    View Slide

  51. Added
    ReAbstractRule class
    >> #checksPackage;
    >> #checksClass;
    >> #checksMethod;
    >> #checksNode
    true?
    Recipe:
    (aClass inheritsFrom: ReAbstractRule) and: [
    selectors anySatisfy: [ :s | aClass perform: s ] ]
    Class Structure

    View Slide

  52. Added

    View Slide

  53. Added
    gtInspectorMethodsIn: composite

    | methods |
    methods := (self methods collect: #asRingDefinition)
    sorted: [ :x :y | x selector < y selector ]
    composite list
    title: 'Methods';
    display: methods;
    format: #selector;
    tags: [ :each | {each methodClass name} ]

    View Slide

  54. Added
    gtInspectorMethodsIn: composite

    | methods |
    methods := (self methods collect: #asRingDefinition)
    sorted: [ :x :y | x selector < y selector ]
    composite list
    title: 'Methods';
    display: methods;
    format: #selector;
    tags: [ :each | {each methodClass name} ]

    View Slide

  55. Added
    gtInspectorMethodsIn: composite

    composite list
    title: 'Methods';
    display: ((self methods collect: #asRingDefinition)
    sorted: [ :x :y | x selector < y selector ]);
    format: #selector;
    tags: [ :each | {each methodClass name} ]

    View Slide

  56. Added
    gtInspectorMethodsIn: composite

    composite list
    title: 'Methods';
    display: [ (self methods collect: #asRingDefinition)
    sorted: [ :x :y | x selector < y selector ] ];
    format: #selector;
    tags: [ :each | {each methodClass name} ]

    View Slide

  57. Added Lazy evaluation
    gtInspectorMethodsIn: composite

    composite list
    title: 'Methods';
    display: [ (self methods collect: #asRingDefinition)
    sorted: [ :x :y | x selector < y selector ] ];
    format: #selector;
    tags: [ :each | {each methodClass name} ]

    View Slide

  58. gtInspectorMethodsIn: composite

    composite list
    title: 'Methods';
    display: [ (self methods collect: #asRingDefinition)
    sorted: [ :x :y | x selector < y selector ] ];
    format: #selector;
    tags: [ :each | {each methodClass name} ]
    Added Lazy evaluation
    Recipe:
    presentation: aPresentation isTheLeftmostRecepientIn: aStatement
    ^ (aStatement isMessage or:
    [ aStatement isCascade ]) and: [
    aStatement leftmostChainReceiver = aPresentation ]

    View Slide

  59. Feedback

    View Slide

  60. Feedback
    http://renraku.inf.usi.ch/rules

    View Slide

  61. Feedback
    http://renraku.inf.usi.ch/rules

    View Slide

  62. Feedback
    http://renraku.inf.usi.ch/rules

    View Slide

  63. Feedback
    http://renraku.inf.usi.ch/rules

    View Slide

  64. Feedback
    http://renraku.inf.usi.ch/rules

    View Slide

  65. @yuriy_tymchuk
    When QualityAssistant Meets Pharo
    Enforced Code Critiques Motivate
    More Valuable Rules
    , Mohammad Ghafari, Oscar Nierstrasz
    Software Composition Group @ University of Bern

    View Slide

  66. @yuriy_tymchuk
    When QualityAssistant Meets Pharo
    Enforced Code Critiques Motivate
    More Valuable Rules
    , Mohammad Ghafari, Oscar Nierstrasz
    Software Composition Group @ University of Bern
    Important
    Fixes

    View Slide

  67. @yuriy_tymchuk
    When QualityAssistant Meets Pharo
    Enforced Code Critiques Motivate
    More Valuable Rules
    , Mohammad Ghafari, Oscar Nierstrasz
    Software Composition Group @ University of Bern
    Important
    Fixes
    Educational
    Rules Removed

    View Slide

  68. @yuriy_tymchuk
    When QualityAssistant Meets Pharo
    Enforced Code Critiques Motivate
    More Valuable Rules
    , Mohammad Ghafari, Oscar Nierstrasz
    Software Composition Group @ University of Bern
    Important
    Fixes
    Educational
    Rules Removed
    Domain Rules
    Added

    View Slide

  69. @yuriy_tymchuk
    When QualityAssistant Meets Pharo
    Enforced Code Critiques Motivate
    More Valuable Rules
    , Mohammad Ghafari, Oscar Nierstrasz
    Software Composition Group @ University of Bern
    Important
    Fixes
    Educational
    Rules Removed
    Domain Rules
    Added
    Migration

    View Slide

  70. @yuriy_tymchuk
    When QualityAssistant Meets Pharo
    Enforced Code Critiques Motivate
    More Valuable Rules
    , Mohammad Ghafari, Oscar Nierstrasz
    Software Composition Group @ University of Bern
    Important
    Fixes
    Educational
    Rules Removed
    Domain Rules
    Added
    Migration
    Private access

    View Slide

  71. @yuriy_tymchuk
    When QualityAssistant Meets Pharo
    Enforced Code Critiques Motivate
    More Valuable Rules
    , Mohammad Ghafari, Oscar Nierstrasz
    Software Composition Group @ University of Bern
    Important
    Fixes
    Educational
    Rules Removed
    Domain Rules
    Added
    Migration
    Private access
    Invocation order

    View Slide

  72. @yuriy_tymchuk
    When QualityAssistant Meets Pharo
    Enforced Code Critiques Motivate
    More Valuable Rules
    , Mohammad Ghafari, Oscar Nierstrasz
    Software Composition Group @ University of Bern
    Important
    Fixes
    Educational
    Rules Removed
    Domain Rules
    Added
    Migration
    Private access
    Invocation order
    Class structure

    View Slide

  73. @yuriy_tymchuk
    When QualityAssistant Meets Pharo
    Enforced Code Critiques Motivate
    More Valuable Rules
    , Mohammad Ghafari, Oscar Nierstrasz
    Software Composition Group @ University of Bern
    Important
    Fixes
    Educational
    Rules Removed
    Domain Rules
    Added
    Migration
    Private access
    Invocation order
    Class structure
    Lazy evaluation

    View Slide