add make targets to lint the code

Fixes #2818

4 files changed, 160 insertions, 46 deletions

diff --git a/.oclint b/.oclint
new file mode 100644
index 00000000..cd68e182
--- /dev/null
+++ b/.oclint
@@ -0,0 +1,8 @@
+rules:
+rule-configurations:
+    # This is the default value (as of the time I wrote this) but I'm making
+    # it explicit since it needs to agree with the value used by clang-format.
+    # Thus, if we ever change the fish style to allow longer lines this should
+    # be changed (as well as the corresponding clang-format config).
+    - key: LONG_LINE
+      value: 100
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index d3e2ca1f..6321cdc7 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -1,15 +1,50 @@
-# Style guide
+# Guidelines For Developers
 
-This is style guide for fish contributors. You should use it for any new code
-that you would add to this project and try to format existing code to use this
-style.
+## Lint Free Code
 
-## Formatting
+Automated analysis tools like cppcheck and oclint can help identify bugs. They also help ensure the code has a consistent style and avoids patterns that tend to confuse people.
+
+Ultimately we want lint free code. However, at the moment a lot of cleanup is required to reach that goal. For now simply try to avoid introducing new lint.
+
+To make linting the code easy there are two make targets: `lint` and `lint-all`. The latter does just what the name implies. The former will lint any modified but not committed `*.cpp` files. If there is no uncommitted work it will lint the files in the most recent commit.
+
+To install the lint checkers on Mac OS X using HomeBrew:
+
+```
+brew tap oclint/formulae
+brew install oclint
+brew install cppcheck
+```
+
+To install the lint checkers on Linux distros that use Apt:
+
+```
+sudo apt-get install clang
+sudo apt-get install oclint
+sudo apt-get install cppcheck
+```
+
+## Fish Script Style Guide
+
+Fish scripts such as those in the *share/functions* and *tests* directories should be formatted using the `fish_indent` command.
+
+Function names should be all lowercase with undescores separating words. Private functions should begin with an underscore. The first word should be `fish` if the function is unique to fish.
+
+The first word of global variable names should generally be `fish` for public vars or `_fish` for private vars to minimize the possibility of name clashes with user defined vars.
+
+## C++ Style Guide
+
+1. The `clang-format` command is authoritative with respect to indentation, whitespace around operators, etc. **Note**: this rule should be ignored at this time. A subsequent commit will add the necessary config file and make targets. After the happens the code will be cleaned up and this rule will become mandatory.
+
+1. All names in code should be `small_snake_case`. No Hungarian notation is used. Classes and structs names should be followed by `_t`.
 
 1. fish uses the Allman/BSD style of indentation.
-2. Indent with spaces, not tabs.
-3. Use 4 spaces per indent (unless needed like `Makefile`).
-4. Opening curly bracket is on the following line:
+
+1. Indent with spaces, not tabs.
+
+1. Use 4 spaces per indent.
+
+1. Opening curly bracket is on the following line:
 
         // ✔:
         struct name
@@ -32,7 +67,7 @@ style.
           // code
         }
 
-5. Put space after `if`, `while` and `for` before conditions.
+1. Put space after `if`, `while` and `for` before conditions.
 
         // ✔:
         if () {}
@@ -40,7 +75,7 @@ style.
         // ✗:
         if() {}
 
-6. Put spaces before and after operators excluding increment and decrement;
+1. Put spaces before and after operators excluding increment and decrement;
 
         // ✔:
         int a = 1 + 2 * 3;
@@ -50,7 +85,7 @@ style.
         int a=1+2*3;
         a ++;
 
-7. Never put spaces between function name and parameters list.
+1. Never put spaces between function name and parameters list.
 
         // ✔:
         func(args);
@@ -58,8 +93,9 @@ style.
         // ✗:
         func (args);
 
-8. Never put spaces after `(` and before `)`.
-9. Always put space after comma and semicolon.
+1. Never put spaces after `(` and before `)`.
+
+1. Always put space after comma and semicolon.
 
         // ✔:
         func(arg1, arg2);
@@ -70,36 +106,3 @@ style.
         func(arg1,arg2);
 
         for (int i = 0;i<LENGTH;i++) {}
-
-## Documentation
-
-Document your code using [Doxygen][dox].
-
-1. Documentation comment should use double star notation or tripple slash:
-
-        // ✔:
-        /// Some var
-        int var;
-
-        /**
-         * Some func
-         */
-        void func();
-
-2. Use slash as tag mark:
-
-        // ✔:
-
-        /**
-         * \param a an integer argument.
-         * \param s a constant character pointer.
-         * \return The results
-         */
-        int foo(int a, const char *s);
-
-## Naming
-
-All names in code should be `small_snake_case`. No Hungarian notation is used.
-Classes and structs names should be followed by `_t`.
-
-[dox]: http://www.stack.nl/~dimitri/doxygen/ "Doxygen homepage"
diff --git a/Makefile.in b/Makefile.in
index 59eb1340..98b8a11c 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -865,6 +865,13 @@ iwyu:
 _iwyu: clean $(PROGRAMS)
 .PHONY: iwyu _iwyu
 
+# Lint the code.
+lint:
+	build_tools/lint.fish $(CXX) $(CXXFLAGS)
+lint-all:
+	build_tools/lint.fish $(CXX) --all $(CXXFLAGS)
+.PHONY: lint lint-all
+
 #
 # Cleanup targets
 #
@@ -891,6 +898,7 @@ clean:
 	rm -f command_list.txt command_list_toc.txt toc.txt
 	rm -f doc_src/index.hdr doc_src/commands.hdr
 	rm -f lexicon_filter lexicon.txt lexicon.log
+	rm -f compile_commands.json xcodebuild.log
 	rm -f FISH-BUILD-VERSION-FILE fish.pc
 	if test "$(HAVE_DOXYGEN)" = 1; then \
 		rm -rf doc user_doc share/man; \
diff --git a/build_tools/lint.fish b/build_tools/lint.fish
new file mode 100755
index 00000000..3fd90b24
--- /dev/null
+++ b/build_tools/lint.fish
@@ -0,0 +1,95 @@
+#!/usr/bin/env fish
+#
+# This is meant to be run by "make lint" or "make lint-all". It is not meant to
+# be run directly from a shell prompt.
+#
+set cppchecks warning,performance,portability,information,missingInclude
+set cppcheck_args
+set c_files
+set all no
+
+set -gx CXX $argv[1]
+set -e argv[1]
+
+if test "$argv[1]" = "--all"
+    set all yes
+    set c_files src/
+    set cppchecks "$cppchecks,unusedFunction"
+    set -e argv[1]
+end
+
+# We only want -D and -I options to be passed thru to cppcheck.
+for arg in $argv
+    if string match -q -- '-D*' $arg
+        set cppcheck_args $cppcheck_args $arg
+    else if string match -q -- '-I*' $arg
+        set cppcheck_args $cppcheck_args $arg
+    end
+end
+if test (uname -m) = "x86_64"
+    set cppcheck_args -D__x86_64__ -D__LP64__ $cppcheck_args
+end
+
+if test $all = no
+    # We haven't been asked to lint all the source. If there are uncommitted
+    # changes lint those, else lint the files in the most recent commit.
+    set pending (git status --porcelain --short --untracked-files=all | sed -e 's/^ *//')
+    if count $pending > /dev/null
+        # There are pending changes so lint those files.
+        for arg in $pending
+            set files $files (string split -m 1 ' ' $arg)[2]
+        end
+    else
+        # No pending changes so lint the files in the most recent commit.
+        set files (git show --porcelain --name-only --pretty=oneline head | tail --lines=+2)
+    end
+
+    # Filter out the non-C/C++ files.
+    set c_files (string match -r '.*\.c(?:pp)?$' -- $files)
+else
+    set c_files src/*.cpp
+end
+
+# We now have a list of files to check so run the linters.
+if set -q c_files[1]
+    if type -q cppcheck
+        echo
+        echo ========================================
+        echo Running cppcheck
+        echo ========================================
+        cppcheck -q --verbose --std=posix --std=c11 --language=c++ \
+            --inline-suppr --enable=$cppchecks $cppcheck_args $c_files
+    end
+
+    if type -q oclint
+        echo
+        echo ========================================
+        echo Running oclint
+        echo ========================================
+        if test (uname -s) = "Darwin"
+            if not test -f compile_commands.json
+                xcodebuild > xcodebuild.log
+                oclint-xcodebuild xcodebuild.log > /dev/null
+            end
+            if test $all = yes
+                oclint-json-compilation-database -e '/pcre2-10.20/' \
+                    -- -enable-global-analysis
+            else
+                set i_files
+                for f in $c_files
+                    set i_files $i_files -i $f
+                end
+                echo oclint-json-compilation-database -e '/pcre2-10.20/' $i_files
+                oclint-json-compilation-database -e '/pcre2-10.20/' $i_files
+            end
+        else
+            # Presumably we're on Linux or other platform not requiring special
+            # handling for oclint to work.
+            oclint $c_files -- $argv
+        end
+    end
+else
+    echo
+    echo 'WARNING: No C/C++ files to check'
+    echo
+end