Objective-C Code Reviews

This Week I’ve been asked to do a code review, to determine the overall health of a existing App code base. Unlike a code walkthrough, where someone familiar with the code shows the main components, this is a code review where I have to examine the project and take notes without any introduction.

The idea is that you might have an application that works great and passes all kinds of acceptance tests, but is built in a such a way that future maintenance and enhancements will be difficult and/or costly. If it is possible to identify problem areas in such an application’s source code, it can help set things on a better course. The sooner potential problems are discovered, the better!

Normally there is only a small budget and not a lot of time. So my pragmatic approach is to do the Review in two phases:  First I take  an initial look at the project to establish a quick opinion of it’s health. After that, I dive deeper into the project, paying extra attention given to the classes that set off the most warning flags during the initial phase.

Phase One: A Quick Look

To get a feel for the overall health do the following things:

• Make sure all external resources required by the project i.e. 3rd-party code are fully contained within the app, or referenced through submodules, or  even better included via CocoaPods or Carthage.
• Open the project in Xcode and build it. Make sure the project builds cleanly with no warnings, and hopefully errors.
• Perform a static analysis in Xcode to see if any other problems show up.
• Run oclint and see if this uncovers any other problems that Xcode’s static analysis didn’t reveal.
• Examine the project structure. Do the various source code files seem to be placed in a reasonable hierarchy? The larger the project is, the more important it becomes to impose some kind of structure, in order to help outsiders find their way around.
• Does the app have unit tests or integration tests? If so, run them and make sure they complete without any failures. Bonus points if tools are in use for measuring test coverage.

This can all be done in several minutes regardless of the app’s size, unless you encounter major problems along the line. If one or two points are not to your liking does not mean that you’ve got a huge problem but you can start to get a “smell” of the project.

OCLint is a fantastic static code analysis tool for improving quality and reducing defects by inspecting C, C++ and Objective-C code. But it is also a very useful tool to locate problems and give you some evidence about code quality.

Using OCLint

I will write about using OCLint with xctool, basically rewording official documentation. OCLint supports all other tools i.e. Xcode IDE, check out documentation for details. Using OCLint with xctool requires 2 steps.

• Generate compile commands database (JSON format) for OCLint using xctool output.
• Run OCLint using generated JSON database.

The first step is straightforward, run xctool the way you would run it normally, the only difference is that you want to create json compilation database. Finally, use oclint-json-compilation-database to run oclint and generate report.

Note: xctool is a replacement for Apple’s xcodebuild that makes it easier to test iOS and Mac products. It’s especially helpful for continuous integration and makes builds much more easy. xctool can be installed from home-brew.

#!/usr/bin/env bash
#
# OCLint script for Xcode
#
# This script will:
# 1. ensure that oclint exists
# 2. use xcodebuild and xcpretty to generate the compile_commands.json
# that oclint requires for understanding the project hierarchy
# 3. run oclint
#

XCODE_PROJECT=<projectfile-name.xcodeproj>
XCODE_SCHEME=<scheme-name>

LINT_EXCLUDES="Libraries|lib|Pods|Carthage|Other Sources"
LINT_REPORT_TYPE="html"

# Adjust rules for objective-c
LINT_LONG_LINE=300
LINT_LONG_VARIABLE_NAME=64
LINT_LONG_METHOD=150

LINT_RULES="-rc LONG_LINE=${LINT_LONG_LINE} \
      -rc LONG_VARIABLE_NAME=${LINT_LONG_VARIABLE_NAME} \
      -rc LONG_METHOD=${LINT_LONG_METHOD}"

# Disable rules for smelling code
LINT_DISABLE_RULES="-disable-rule=UnusedMethodParameter \
            -disable-rule=UselessParentheses \
            -disable-rule=IvarAssignmentOutsideAccessorsOrInit"

# Threshold
LINT_PRIORITY_1_THRESHOLD=0
LINT_PRIORITY_2_THRESHOLD=10
LINT_PRIORITY_3_THRESHOLD=20
LINT_THRESHOLD="-max-priority-1=${LINT_PRIORITY_1_THRESHOLD} \
    -max-priority-2=${LINT_PRIORITY_2_THRESHOLD} \
    -max-priority-3=${LINT_PRIORITY_3_THRESHOLD}"

# Reports
LINT_REPORTS_DIR=oclint-reports
LINT_REPORT_FILE=oclint.html
rm -rf ${LINT_REPORT_DIR}
mkdir -p ${LINT_REPORTS_DIR}

export PATH="/usr/local/bin:$PATH"

if ! hash oclint 2>/dev/null; then
  echo "oclint not found, analyzing stopped"
  exit 1
fi

echo "[OCLint] Preparing..."

echo "[OCLint] Cleaning Xcode project..."
xcodebuild -scheme ${XCODE_SCHEME} clean
rm compile_commands.json

echo "[OCLint] Creating compile_commands.json..."
xctool -project ${XCODE_PROJECT} -scheme ${XCODE_SCHEME} -reporter json-compilation-database:compile_commands.json

echo "[OCLint] Linting..."
oclint-json-compilation-database \
  -exclude ${LINT_EXCLUDES} \
  -- \
  -report-type ${LINT_REPORT_TYPE} \
  ${LINT_RULES} \
  ${LINT_DISABLE_RULES} \
  ${LINT_THRESHOLD} \
  -o ${LINT_REPORTS_DIR}/${LINT_REPORT_FILE} \
  -stats \
  -verbose \
  -list-enabled-rules \