STATIC CODE ANALYSIS
and
MANUAL CODE REVIEW
Jakub Papcun
Jan Svoboda
HELLO!
Jakub Papcun, Jan Svoboda
• FI MUNI graduates
• Java Developers since 2011
• Scrum Masters
How Developers See Quality
CODE QUALITY
Code
Quality
Readable
Documented
Secure
Bug Free
THERE IS NO
PERFECT CODE
WHAT IS STATIC CODE ANALYSIS
Analysis of computer software
performed without executing the
software.
ADVANTAGES
 No program execution
 Automated process
 Possible to run as part of Continuous
Integration
1
TYPES OF STATIC CODE
ANALYSIS
• correct assignment of types of objectsType Checking
• style of the code and its formattingStyle Checking
• helps user make sense of large codebase and may
include refactoring capabilities
Program
Understanding
• uses dataflow analysis for detection of possible code
injectionSecurity review
• looks for places in the code where program may behave
in a different way from the way intended by developerBug Finding
MCR IN DEVELOPMENT CYCLE
Software
Dev.
Cycle
Architecture
Design
ImplementationTesting
Deployment
Implementation
WHY USE STATIC CODE
ANALYSIS
Higher Code Quality
Readibility No Input
Cheaper defect fixing
Education
Coding
Quidelines
Compliance
Repeatibility
DRAWBACKS
Only
STATIC
analysis
False
sense
of
security
Possible
overhead
Time
consuming
if done
manually
PITFALLS OF STATIC CODE
ANALYSIS
Is a problem Is NOT a problem
Was found True Positive False Positive
Was NOT found False Negative
YOU CAN LEARN A LOT
ABOUT YOUR CODE
LEARN ABOUT YOUR CODE
 Lines of Code (LOC)
 Comments Quality
 Code Duplication
 Technical Debt
 Cyclomatic Complexity
 Dependency Cycle Detection
CHECKERS
Rule defining possible bug/defect
Unused local variable
Memory leaks
SQL injection
Call of function on null
SEVERITIES – Level 1
 Very serious problems
 May crash at runtime
 Null pointer dereference
 SQL connection not closed
 Buffer overflow
SEVERITIES – Level 1
SEVERITIES – Level 2
 Security issues
 Crash or corrupt the system
 Modification of unmodifiable collection
 Data/SQL Injection
 Memory leaks
SEVERITIES – Level 2
SEVERITIES – Level 3
 Moderate issues
 Do not usually crush running program
 Unused private method
 Possible error in bit operations
 Incorrect allocation size
SEVERITIES – Level 3
SEVERITIES – Level 4
 Coding standards, Performance issues
 Comparing objects with ==
 Empty catch clause
 Statemenet has no effect
SEVERITIES – Level 4
EXCERSISE
Can return null
A NullPointerException is thrown in case of an attempt to dereference a null value.
EXCERSISE
Statement always false
1. Statement is always false and never enters the block
s is always null
2. s variable is always null and NullPointerException may be thrown
EXCERSISE
& or &&
Questionable use of bit operation ‘&’ in expression. Did you mean ‘&&’?
EXCERSISE
j is never used
1. j variable is never used and thus redundant
k not initialized
2. k variable is never initialized and thus unusable
EXCERSISE
REST may fail and return null
may return null
THANKS!
Any questions?
MANUAL CODE REVIEW
Systematic examination of the
source code
WHY?
Defect finding
MCR IN DEVELOPMENT CYCLE
Software
Dev.
Cycle
Architecture
Design
ImplementationTesting
Deployment
Implementation
ADVANTAGES
 Different point of view
 Decreases cost of defect fixing
 Education
COST OF DEFECT FIX
COST OF DEFECT FIX
WHAT MAKES GOOD CODE
REVIEW?
 Goal
 People
 Technical knowledge
TYPES OF MCR
 Formal
 Informal
 Tool-assisted
FORMAL CODE REVIEW
 Typically face-to-face meeting
 Roles (moderator, observer, reviewer)
 Participants go through the source code to
fulfill goal of review
• Well documented
• Process orientedPros
• Time consuming
• Effort required does not correspond to value gained
• Human Factor
Cons
INFORMAL CODE REVIEW
 Typically two developers (author and
reviewer) conducting ad-hoc review
 Over-the-shoulder review
 Extreme programming
• Simple
• Saves time and resourcesPros
• Not documented
• Not process orientedCons
TOOL-ASSISTED CODE REVIEW
 A tool is used for the review
 Designed to mitigate drawbacks of other
approaches
• Documented
• Enforcing process
• Time efficient
• Reviewer has all the time required
Pros
• Cost of the tool
• It is easier for reviewer to cheatCons
CODE REVIEW TOOL FEATURES
Automated File
Gathering
Combined
Display
Automated
Metrics
Collection
Process
Enforcement
ATLASSIAN STASH
It is better to make Code review natural part of
development process
GIT WORKFLOW
RELATION TO STATIC CODE
ANALYSIS?
Establish
Goal
Run
SCA
Review
Code
Fix Code
HUMAN FACTOR
The only factor that ruins manual code review
LET’S SUM UP!
Effective Code Review
Choose
proper
review
method
Establish
the goal
Be
honest
Use
proper
and
polite
language
Never be
personal
THANKS!
Any questions?