Merge pull request #1 from jack-ullery/linter

Add Linter

.clang-tidy

@@ -0,0 +1,33 @@
+---
+Checks: 'clang-diagnostic-*,
+         clang-analyzer-*,
+         -*,
+         bugprone-*,
+         cert-*,
+         clang-analyzer-*,
+         concurrency-*,
+         cppcoreguidelines-*,
+         google-*,
+         hicpp-*,
+         linuxkernel-*,
+         llvm-*,
+         misc-*,
+         performance-*,
+         portability-*,
+         readability-*,
+         -bugprone-easily-swappable-parameters,
+         -readability-identifier-length,
+         -google-runtime-int,
+         -llvm-header-guard*,
+         -misc-const-correctness,
+         -misc-no-recursion'
+WarningsAsErrors: ''
+HeaderFilterRegex: './src/.*hh'
+AnalyzeTemporaryDtors: false
+FormatStyle:     none
+CheckOptions:
+  - key:    misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
+    value:  'true'
+
+...
+

CMakeLists.txt

@@ -61,6 +61,52 @@ FLEX_TARGET(LEXER
 
 ADD_FLEX_BISON_DEPENDENCY(LEXER PARSER)
 
+#### Linter and Static Analysis ####
+find_program(CLANG_TIDY NAMES clang-tidy)
+find_program(CPPCHECK NAMES cppcheck)
+
+# If cmake was compiled with DANALYZE=TRUE
+string(TOLOWER "${ANALYZE}" ANALYZE)
+if("${ANALYZE}" STREQUAL "true")
+  message(STATUS "Adding static-analysis tools to build:")
+  set(USE_CLANG_TIDY TRUE)
+  set(USE_CPPCHECK TRUE)
+  unset(ANALYZE CACHE)
+endif()
+
+# If we want to use clang-tidy
+if(${USE_CLANG_TIDY})
+  if(${CLANG_TIDY} STREQUAL "CLANG_TIDY-NOTFOUND")
+    message(WARNING "Could not find clang-tidy installation. Please install clang-tidy if you want to use it for static analysis.")
+  else()
+    message(STATUS "Adding clang-tidy to build")
+    set(CMAKE_CXX_CLANG_TIDY clang-tidy; --config-file ${CMAKE_CURRENT_BINARY_DIR}/.clang-tidy)
+  endif()
+  unset(USE_CLANG_TIDY CACHE)
+endif()
+
+add_custom_target(echo
+  COMMAND echo test["$<$<CONFIG:Release>:yes>"]
+  COMMAND_EXPAND_LISTS
+)
+
+# If we want to use cppcheck
+if(${USE_CPPCHECK})
+  if(${CPPCHECK} STREQUAL "CPPCHECK-NOTFOUND")
+    message(WARNING "Could not find cppcheck installation. Please install cppcheck if you want to use it for static analysis.")
+  else()
+    message(STATUS "Adding cppcheck to build")
+    set(CMAKE_CXX_CPPCHECK cppcheck
+      --enable=warning,performance,portability,information 
+      --suppress=missingInclude --suppress=unmatchedSuppression --suppress=internalAstError 
+      --quiet
+      --inline-suppr
+      ${SOURCES}
+    )
+  endif()
+  unset(USE_CPPCHECK CACHE)
+endif()
+
 #### Set Compiler Options ####
 set(CMAKE_CXX_FLAGS "-g -Wall -Wextra")
 set(CMAKE_CXX_STANDARD 17)
@@ -69,12 +115,25 @@ set(CMAKE_CXX_STANDARD 17)
 
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
 
-#### Create the library ####
-add_library(${LIBRARY_NAME} ${SOURCES} ${FLEX_LEXER_OUTPUTS} ${BISON_PARSER_OUTPUT_SOURCE})
+#### Compile the autogenerated code ####
+add_library(autogenerated_sources OBJECT ${FLEX_LEXER_OUTPUTS} ${BISON_PARSER_OUTPUT_SOURCE})
+
+target_include_directories(autogenerated_sources PRIVATE ${PROJECT_SOURCE_DIR}/parser)
+target_include_directories(autogenerated_sources SYSTEM PRIVATE ${AUTOGEN_SOURCE_DIR})
+
+# Disable clang-tidy and cppcheck for this target
+set_target_properties(autogenerated_sources 
+                      PROPERTIES
+                        CXX_CLANG_TIDY ""
+                        CXX_CPPCHECK ""
+                      )
+
+#### Create the main library ####
+add_library(${LIBRARY_NAME} ${SOURCES} $<TARGET_OBJECTS:autogenerated_sources>)
 
 target_include_directories(${LIBRARY_NAME} PUBLIC  ${PROJECT_SOURCE_DIR})
 target_include_directories(${LIBRARY_NAME} PRIVATE ${PROJECT_SOURCE_DIR}/parser)
-target_include_directories(${LIBRARY_NAME} PRIVATE ${AUTOGEN_SOURCE_DIR})
+target_include_directories(${LIBRARY_NAME} SYSTEM PRIVATE ${AUTOGEN_SOURCE_DIR})
 
 # Create target to install library
 install(TARGETS ${LIBRARY_NAME} DESTINATION lib)

README.md

@@ -15,7 +15,7 @@ Packages needed to compile the library:
 sudo apt install pkg-config cmake clang bison flex libfl-dev
 ```
 
-### Testing
+### Testing (Optional)
 Additional packages needed to run the tests:
 * GoogleTest
 * GoogleMock
@@ -25,6 +25,16 @@ Additional packages needed to run the tests:
 sudo apt install libgtest-dev libgmock-dev
 ```
 
+### Linters and Static Analysis (Optional)
+Optional packages needed to run linters and static analysis checks:
+* clang-tidy
+* cppcheck
+
+#### Install commands (Ubuntu)
+```
+sudo apt install clang-tidy-15 cppcheck
+```
+
 ## Compilation Instructions
 ### Prebuild
 If you want to run the tests, first you must load the example profiles from the main [apparmor](https://gitlab.com/apparmor/apparmor/-/tree/master/parser/tst/simple_tests) repository. These profiles are included in a submodule for convenience.
@@ -37,6 +47,11 @@ Before you build the library, you should first generate the makefile by running:
 cmake .
 ```
 
+Optionally, if you want to run linters and static analysis checks:
+```
+cmake -DANALYZE=TRUE .
+```
+
 ### Build
 After the makefile is generated, you can build the library:
 ```

src/apparmor_file_rule.cc

@@ -1,8 +1,10 @@
+#include <utility>
+
 #include "apparmor_file_rule.hh"
 #include "parser/tree/FileNode.hh"
 
 AppArmor::FileRule::FileRule(std::shared_ptr<FileNode> model)
-  : model{model}
+  : model{std::move(model)}
 {   }
 
 std::string AppArmor::FileRule::getFilename() const

src/apparmor_file_rule.hh

@@ -10,7 +10,7 @@ namespace AppArmor {
   class FileRule {
     public:
       FileRule() = default;
-      FileRule(std::shared_ptr<FileNode> model);
+      explicit FileRule(std::shared_ptr<FileNode> model);
 
       std::string getFilename() const;
       std::string getFilemode() const;
@@ -21,6 +21,6 @@ namespace AppArmor {
     private:
       std::shared_ptr<FileNode> model;
   };
-}
+} // namespace AppArmor
 
 #endif // APPARMOR_FILE_RULE_HH

src/apparmor_parser.hh

@@ -11,7 +11,7 @@ class ParseTree;
 namespace AppArmor {
   class Parser {
     public:
-      Parser(std::ifstream &stream);
+      explicit Parser(std::ifstream &stream);
 
       std::list<Profile> getProfileList() const;
 
@@ -20,6 +20,6 @@ namespace AppArmor {
 
       std::list<Profile> profile_list; 
   };
-}
+} // namespace AppArmor
 
 #endif // APPARMOR_PARSER_HH

src/apparmor_profile.hh

@@ -12,7 +12,7 @@ class ProfileNode;
 namespace AppArmor {
   class Profile {
     public:
-      Profile(std::shared_ptr<ProfileNode> profile_model);
+      explicit Profile(std::shared_ptr<ProfileNode> profile_model);
 
       // Returns the name of this profile
       std::string name() const;
@@ -26,6 +26,6 @@ namespace AppArmor {
     private:
       std::shared_ptr<ProfileNode> profile_model;
   };
-}
+} // namespace AppArmor
 
 #endif // APPARMOR_PROFILE_HH

src/parser/lexer.hh

@@ -15,12 +15,13 @@
 
 class Lexer : public yyFlexLexer {
   public:
-    Lexer(std::istream& arg_yyin)
+    explicit Lexer(std::istream& arg_yyin)
       : yyFlexLexer(arg_yyin, std::cout) {}
-  
+
     Lexer(std::istream& arg_yyin, std::ostream& arg_yyout)
       : yyFlexLexer(arg_yyin, arg_yyout) {}
-
+
+    // NOLINTNEXTLINE
     virtual symbol_type yylex(Driver& driver);
 };
 

src/parser/lib.c

@@ -20,13 +20,14 @@
 #include "parser.h"
 
 #include <ctype.h>
-#include <stdint.h>
 #include <stdbool.h>
+#include <stdint.h>
 #include <string.h>
 
 #include <sys/stat.h>
 #include <sys/types.h>
 
+//NOLINTBEGIN
 /**
  * isodigit - test if a character is an octal digit
  * @c: character to test
@@ -179,3 +180,4 @@ int strn_escseq(const char **pos, const char *chrs, size_t n)
 	pos--;
 	return -1;
 }
+//NOLINTEND

src/parser/parser.cc

@@ -20,22 +20,23 @@
 
 /* assistance routines */
 
-#include <assert.h>
-#include <ctype.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <stdarg.h>
-#include <string.h>
+#include <cassert>
+#include <cctype>
+#include <cstdarg>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <fcntl.h>
 #include <linux/capability.h>
-#include <sys/types.h>
 #include <sys/stat.h>
-#include <fcntl.h>
+#include <sys/types.h>
 #include <unistd.h>
 
 #include "lib.h"
 #include "common.hh"
 #include "lexer.hh"
 
+// NOLINTBEGIN
 /* #define DEBUG */
 #ifdef DEBUG
 #undef PDEBUG
@@ -230,3 +231,4 @@ char *processid(const char *string, int len)
 		return processquoted(string, len);
 	return processunquoted(string, len);
 }
+// NOLINTEND

src/parser/parser.h

@@ -23,8 +23,8 @@
 #define __AA_PARSER_H
 
 #include <endian.h>
-#include <string.h>
 #include <stdint.h>
+#include <string.h>
 #include <sys/resource.h>
 
 #include <libintl.h>

src/parser/tree/AbstractionNode.hh

@@ -12,10 +12,10 @@ class AbstractionNode : public RuleNode {
     std::string getPath();
 
   private:
-    virtual operator std::string() const;
+    virtual explicit operator std::string() const;
 
     std::string path;
-    bool is_if_exists;
+    bool is_if_exists = false;
 };
 
 #endif // ABSTRACTION_NODE_HH

src/parser/tree/AliasNode.cc

@@ -4,8 +4,7 @@
 #include <sstream>
 
 AliasNode::AliasNode(const std::string &from, const std::string &to)
-  : TreeNode(),
-    from{from},
+  : from{from},
     to{to}
 {   }
 

src/parser/tree/AliasNode.hh

@@ -9,7 +9,7 @@ class AliasNode : public TreeNode {
     AliasNode(const std::string &from, const std::string &to);
 
   private:
-    virtual operator std::string() const;
+    virtual explicit operator std::string() const;
     std::string from;
     std::string to;
 };

src/parser/tree/FileNode.hh

@@ -19,7 +19,7 @@ class FileNode : public RuleNode {
     std::string getFilemode() const;
 
   private:
-    bool isSubset;
+    bool isSubset = false;
     std::string filename;
     std::string exec_target;
     std::string fileMode;

src/parser/tree/LinkNode.hh

@@ -9,10 +9,10 @@ class LinkNode : public RuleNode {
     LinkNode() = default;
     LinkNode(uint64_t startPos, uint64_t stopPos, bool isSubset, const std::string &linkFrom, const std::string &linkTo);
 
-    virtual operator std::string() const;
+    virtual explicit operator std::string() const;
 
   private:
-    bool isSubset;
+    bool isSubset = false;
     std::string from;
     std::string to;
 };

src/parser/tree/ParseTree.cc

@@ -1,7 +1,9 @@
+#include <utility>
+
 #include "ParseTree.hh"
 #include "TreeNode.hh"
 
 ParseTree::ParseTree(TreeNode preamble, std::shared_ptr<std::list<ProfileNode>> profileList)
-  : preamble{preamble}, 
-    profileList{profileList}
+  : preamble{std::move(preamble)}, 
+    profileList{std::move(profileList)}
 {   }

src/parser/tree/PrefixNode.cc

@@ -2,8 +2,7 @@
 #include "tree/TreeNode.hh"
 
 PrefixNode::PrefixNode(bool audit, bool should_deny, bool owner)
-  : TreeNode(),
-    audit{audit},
+  : audit{audit},
     should_deny{should_deny},
     owner{owner}
 {   }

src/parser/tree/PrefixNode.hh

@@ -6,7 +6,7 @@
 
 class PrefixNode : public TreeNode {
   public:
-    PrefixNode(bool audit = DEFAULT_AUDIT, bool should_deny = DEFAULT_PERM_MODE, bool owner = DEFAULT_OWNER);
+    explicit PrefixNode(bool audit = DEFAULT_AUDIT, bool should_deny = DEFAULT_PERM_MODE, bool owner = DEFAULT_OWNER);
 
     static constexpr bool DEFAULT_AUDIT       = false; 
     static constexpr bool DEFAULT_PERM_MODE   = false;

src/parser/tree/ProfileNode.hh

@@ -13,7 +13,7 @@ class ProfileNode : public TreeNode {
 
     RuleList<ProfileNode> getRules();
 
-  protected:
+  private:
     RuleList<ProfileNode> rules;
 };