Enable and fix clang-tidy for headers

.clang-tidy

@@ -1,3 +1,4 @@
-Checks: 'clang-analyzer-*,bugprone-*,performance-*,readability-*,modernize-*,-bugprone-easily-swappable-parameters,-bugprone-assignment-in-if-condition,-bugprone-narrowing-conversions,-bugprone-implicit-widening-of-multiplication-result,-bugprone-branch-clone,-bugprone-string-literal-with-embedded-nul,-bugprone-macro-parentheses,-bugprone-exception-escape,-performance-avoid-endl,-readability-identifier-length,-readability-named-parameter,-readability-implicit-bool-conversion,-readability-magic-numbers,-readability-else-after-return,-readability-function-cognitive-complexity,-readability-avoid-nested-conditional-operator,-readability-convert-member-functions-to-static,-readability-container-size-empty,-readability-use-anyofallof,-modernize-use-trailing-return-type,-modernize-use-auto,-modernize-return-braced-init-list'
+Checks: 'clang-analyzer-*,bugprone-*,performance-*,readability-*,modernize-*,-bugprone-easily-swappable-parameters,-bugprone-assignment-in-if-condition,-bugprone-narrowing-conversions,-bugprone-implicit-widening-of-multiplication-result,-bugprone-branch-clone,-bugprone-string-literal-with-embedded-nul,-bugprone-macro-parentheses,-bugprone-exception-escape,-bugprone-reserved-identifier,-performance-avoid-endl,-performance-enum-size,-readability-identifier-length,-readability-named-parameter,-readability-implicit-bool-conversion,-readability-magic-numbers,-readability-else-after-return,-readability-function-cognitive-complexity,-readability-avoid-nested-conditional-operator,-readability-convert-member-functions-to-static,-readability-container-size-empty,-readability-use-anyofallof,-modernize-use-trailing-return-type,-modernize-use-auto,-modernize-return-braced-init-list,-modernize-use-nodiscard,-modernize-macro-to-enum'
 WarningsAsErrors: '*'
 FormatStyle: 'file'
+HeaderFilterRegex: '^lib/.*\.h$'

.github/workflows/codeql-analysis.yml

@@ -56,7 +56,7 @@ jobs:
     - name: install dependencies
       run: |
         sudo apt-get update
-        sudo apt-get install -y libpoppler-dev libpoppler-glib-dev libcurl4-openssl-dev
+        sudo apt-get install -y libpoppler-dev libpoppler-glib-dev libcairo2-dev libglib2.0-dev libjpeg-dev libcurl4-openssl-dev zlib1g-dev
     - name: make
       run: make -j$(nproc)
 

lib/argget.h

@@ -30,15 +30,15 @@ class SwitchArgBase : public ArgBase
 public:
   SwitchArgBase(std::initializer_list<std::string> names,
                 std::string doc, std::string errorHint="")
-  : _names(names), _doc(doc), _errorHint(errorHint)
+  : _names(names), _doc(std::move(doc)), _errorHint(std::move(errorHint))
   {
     if(names.size() < 1)
     {
       throw(std::logic_error("No parameter definitions"));
     }
   }
 
-  bool parse(std::list<std::string>& argv)
+  bool parse(std::list<std::string>& argv) override
   {
     if(match(argv))
     {
@@ -67,7 +67,7 @@ class SwitchArgBase : public ArgBase
 
   virtual bool get_value(std::list<std::string>&) = 0;
 
-  std::string docName() const
+  std::string docName() const override
   {
     std::stringstream docName;
     std::string sep = ", ";
@@ -109,13 +109,13 @@ class SwitchArg : public SwitchArgBase
   SwitchArg& operator=(const SwitchArg&) = delete;
 
   SwitchArg(T& value, std::initializer_list<std::string> names, std::string doc)
-  : SwitchArgBase(names, doc), _value(value)
+  : SwitchArgBase(names, std::move(doc)), _value(value)
   {}
 
 private:
-  bool get_value(std::list<std::string>& argv)
+  bool get_value(std::list<std::string>& argv) override
   {
-    if(std::is_same<T, bool>::value)
+    if(std::is_same_v<T, bool>)
     {
       _value = true;
       return true;
@@ -129,30 +129,30 @@ class SwitchArg : public SwitchArgBase
     return false;
   }
 
-  std::string typeHint() const
+  std::string typeHint() const override
   {
-    if(std::is_same<T, bool>::value)
+    if(std::is_same_v<T, bool>)
     {
       return "";
     }
-    else if(std::is_same<T, std::string>::value)
+    else if(std::is_same_v<T, std::string>)
     {
       return " <string>";
     }
-    else if(std::is_integral<T>::value)
+    else if(std::is_integral_v<T>)
     {
       return " <int>";
     }
   }
 
-  void convert(std::string& res, std::string s) {res = s;}
-  void convert(bool& res, std::string) {res = true;}
-  void convert(int& res, std::string s) {res = std::stoi(s);}
-  void convert(long& res, std::string s) {res = std::stol(s);}
-  void convert(long long& res, std::string s) {res = std::stoll(s);}
-  void convert(unsigned int& res, std::string s) {res = std::stoul(s);} // Silly C++ has no stou
-  void convert(unsigned long& res, std::string s) {res = std::stoul(s);}
-  void convert(unsigned long long& res, std::string s) {res = std::stoull(s);}
+  void convert(std::string& res, const std::string& s) {res = s;}
+  void convert(bool& res, const std::string&) {res = true;}
+  void convert(int& res, const std::string& s) {res = std::stoi(s);}
+  void convert(long& res, const std::string& s) {res = std::stol(s);}
+  void convert(long long& res, const std::string& s) {res = std::stoll(s);}
+  void convert(unsigned int& res, const std::string& s) {res = std::stoul(s);} // Silly C++ has no stou
+  void convert(unsigned long& res, const std::string& s) {res = std::stoul(s);}
+  void convert(unsigned long long& res, const std::string& s) {res = std::stoull(s);}
 
   T& _value;
 };
@@ -161,24 +161,24 @@ template<typename T>
 class EnumSwitchArg : public SwitchArgBase
 {
 public:
-  typedef std::map<std::string, T> Mappings;
-  typedef std::map<T, std::string> BackMappings;
+  using Mappings = std::map<std::string, T>;
+  using BackMappings = std::map<T, std::string>;
 
   EnumSwitchArg() = delete;
   EnumSwitchArg(const EnumSwitchArg&) = delete;
   EnumSwitchArg& operator=(const EnumSwitchArg&) = delete;
 
-  EnumSwitchArg(T& value, Mappings mappings, std::initializer_list<std::string> names,
+  EnumSwitchArg(T& value, const Mappings& mappings, std::initializer_list<std::string> names,
                 std::string doc, std::string errorHint="")
-  : SwitchArgBase(names, doc, errorHint), _value(value), _mappings(mappings)
+  : SwitchArgBase(names, std::move(doc), std::move(errorHint)), _value(value), _mappings(mappings)
   {
     for(const auto& [k, v] : mappings)
     {
       _backMappings[v] = k;
     }
   }
 
-  std::string typeHint() const
+  std::string typeHint() const override
   {
     return " <choice>";
   }
@@ -190,7 +190,7 @@ class EnumSwitchArg : public SwitchArgBase
 
 private:
 
-  bool get_value(std::list<std::string>& argv)
+  bool get_value(std::list<std::string>& argv) override
   {
     if(!argv.empty())
     {
@@ -218,10 +218,10 @@ class PosArg : public ArgBase
   PosArg& operator=(const PosArg&) = delete;
 
   PosArg(std::string& value, std::string name, bool optional=false)
-  : _value(value), _name(name), _optional(optional)
+  : _value(value), _name(std::move(name)), _optional(optional)
   {}
 
-  bool parse(std::list<std::string>& argv)
+  bool parse(std::list<std::string>& argv) override
   {
     if(!argv.empty())
     {
@@ -233,7 +233,7 @@ class PosArg : public ArgBase
     return _optional;
   }
 
-  std::string docName() const
+  std::string docName() const override
   {
     return _optional ? "["+_name+"]" : "<"+_name+">";
   }
@@ -254,7 +254,7 @@ friend class SubArgGet;
   ArgGet& operator=(const ArgGet&) = default;
 
   ArgGet(std::list<SwitchArgBase*> argDefs, std::list<PosArg*> posArgDefs = {})
-  : _argDefs(argDefs), _posArgDefs(posArgDefs)
+  : _argDefs(std::move(argDefs)), _posArgDefs(std::move(posArgDefs))
   {}
 
   bool get_args(int argc, char** argv)
@@ -269,7 +269,7 @@ friend class SubArgGet;
 
     for(int i = 1; i < argc; i++)
     {
-      argList.push_back(argv[i]);
+      argList.emplace_back(argv[i]);
     }
 
     bool progress = true;
@@ -388,12 +388,13 @@ class SubArgGet
   SubArgGet(const SubArgGet&) = delete;
   SubArgGet& operator=(const SubArgGet&) = delete;
 
-  SubArgGet(std::list<std::pair<std::string,
-                                std::pair<std::list<SwitchArgBase*>, std::list<PosArg*>>>> map)
+  SubArgGet(const std::list<std::pair<std::string,
+                                      std::pair<std::list<SwitchArgBase*>,
+                                                std::list<PosArg*>>>>& map)
   {
     for(const auto& [subCommand, args] : map)
     {
-      _order.push_back(subCommand);
+      _order.emplace_back(subCommand);
       _subCommands[subCommand] = ArgGet(args.first, args.second);
     }
   }
@@ -410,7 +411,7 @@ class SubArgGet
 
     for(int i = 1; i < argc; i++)
     {
-      argList.push_back(argv[i]);
+      argList.emplace_back(argv[i]);
     }
 
     if(argc < 2)
@@ -435,7 +436,7 @@ class SubArgGet
     return true;
   }
 
-  std::string argHelp(std::string subCommand="")
+  std::string argHelp(const std::string& subCommand="")
   {
     std::stringstream help;
 
@@ -446,7 +447,7 @@ class SubArgGet
     }
     else
     {
-      for(std::string subCommand : _order)
+      for(const std::string& subCommand : _order)
       {
         help << _name << " " << subCommand << _subCommands.at(subCommand)._argHelp() << std::endl;
       }

lib/binfile.h

@@ -12,7 +12,7 @@ class InBinFile
   InBinFile(const InBinFile&) = delete;
   InBinFile& operator=(const InBinFile&) = delete;
 
-  InBinFile(std::string name)
+  InBinFile(const std::string& name)
   {
     if(name == "-")
     {
@@ -60,7 +60,7 @@ class OutBinFile
 
   using value_type = std::ostream&;
 
-  OutBinFile(std::string name)
+  OutBinFile(const std::string& name)
   {
     if(name == "-")
     {

lib/converter.h

@@ -28,18 +28,20 @@ class Converter
     return converter;
   }
 
-  typedef std::pair<std::string, std::string> ConvertKey;
-  typedef std::function<Error(std::string inFileName, const IppPrintJob& job,
-                              WriteFun writeFun, ProgressFun progressFun)> ConvertFun;
+  using ConvertKey = std::pair<std::string, std::string>;
+  using ConvertFun = std::function<Error(const std::string& inFileName, const IppPrintJob& job,
+                                         const WriteFun& writeFun, const ProgressFun& progressFun)>;
 
   ConvertFun Pdf2Printable =
-    [](std::string inFileName, const IppPrintJob& job, WriteFun writeFun, ProgressFun progressFun)
+    [](const std::string& inFileName, const IppPrintJob& job,
+       const WriteFun& writeFun, const ProgressFun& progressFun)
     {
       return pdf_to_printable(inFileName, job.printParams, writeFun, progressFun);
     };
 
   ConvertFun Baselinify =
-    [](std::string inFileName, const IppPrintJob&, WriteFun writeFun, ProgressFun progressFun)
+    [](const std::string& inFileName, const IppPrintJob&,
+       const WriteFun& writeFun, const ProgressFun& progressFun)
     {
       InBinFile in(inFileName);
       if(!in)
@@ -56,7 +58,8 @@ class Converter
     };
 
   ConvertFun JustUpload =
-    [](std::string inFileName, const IppPrintJob&, WriteFun writeFun, ProgressFun progressFun)
+    [](const std::string& inFileName, const IppPrintJob&,
+       const WriteFun& writeFun, const ProgressFun& progressFun)
     {
       InBinFile in(inFileName);
       if(!in)
@@ -70,7 +73,8 @@ class Converter
     };
 
   ConvertFun FixupText =
-  [](std::string inFileName, const IppPrintJob&, WriteFun writeFun, ProgressFun progressFun)
+  [](const std::string& inFileName, const IppPrintJob&,
+     const WriteFun& writeFun, const ProgressFun& progressFun)
   {
     InBinFile in(inFileName);
     if(!in)
@@ -100,7 +104,8 @@ class Converter
      {{MiniMime::JPEG, MiniMime::JPEG}, Baselinify},
      {{"text/plain", "text/plain"}, FixupText}};
 
-  std::optional<ConvertFun> getConvertFun(std::string inputFormat, std::string targetFormat)
+  std::optional<ConvertFun> getConvertFun(const std::string& inputFormat,
+                                          const std::string& targetFormat)
   {
     for(const auto& [convertKey, convertFun] : Pipelines)
     {
@@ -116,13 +121,13 @@ class Converter
     return {};
   }
 
-  bool canConvert(std::string inputFormat, std::string targetFormat)
+  bool canConvert(const std::string& inputFormat, const std::string& targetFormat)
   {
     return (bool)getConvertFun(inputFormat, targetFormat);
   }
 
-  std::optional<std::string> getTargetFormat(std::string inputFormat,
-                                             List<std::string> supportedFormats)
+  std::optional<std::string> getTargetFormat(const std::string& inputFormat,
+                                             const List<std::string>& supportedFormats)
   {
     for(const auto& [convertKey, convertFun] : Pipelines)
     {
@@ -138,7 +143,7 @@ class Converter
     return {};
   }
 
-  List<std::string> possibleInputFormats(List<std::string> supportedFormats)
+  List<std::string> possibleInputFormats(const List<std::string>& supportedFormats)
   {
     List<std::string> inputFormats;
     for(const auto& [convertKey, convertFun] : Pipelines)

lib/curlrequester.h

@@ -68,7 +68,7 @@ class CurlRequester
   Bytestream _data;
 
   CURL* _curl;
-  struct curl_slist* _opts = NULL;
+  struct curl_slist* _opts = nullptr;
 
   z_stream _zstrm;
   Compression _nextCompression = Compression::None;
@@ -80,7 +80,7 @@ class CurlIppPosterBase : public CurlRequester
 {
 public:
   ~CurlIppPosterBase();
-  virtual CURLcode await(Bytestream* = nullptr);
+  CURLcode await(Bytestream* = nullptr) override;
 
   bool write(Bytestream&& data);
   size_t requestWrite(char* dest, size_t size);

lib/error.h

@@ -4,6 +4,6 @@
 #include <optional>
 #include <string>
 
-typedef std::optional<std::string> Error;
+using Error = std::optional<std::string>;
 
 #endif // ERROR_H

lib/functions.h

@@ -5,8 +5,8 @@
 
 #include <functional>
 
-typedef std::function<bool(Bytestream&& data)> WriteFun;
-typedef std::function<void(size_t page, size_t total)> ProgressFun;
+using WriteFun = std::function<bool(Bytestream&& data)>;
+using ProgressFun = std::function<void(size_t page, size_t total)>;
 
 static ProgressFun noOpProgressfun([](size_t, size_t) -> void {});
 

lib/ippattr.h

@@ -120,7 +120,7 @@ class IppAttr : public IppValue
   IppOneSetOf asList() const;
 
   template <typename T>
-  IppAttr(IppTag tag, T value) : IppValue(value), _tag(tag) {}
+  IppAttr(IppTag tag, const T& value) : IppValue(value), _tag(tag) {}
 
   IppTag tag() const {return _tag;}
   IppValue value() const {return *this;}
@@ -153,7 +153,7 @@ struct IppAttrs: public std::map<std::string, IppAttr>
   }
 
   template <typename T>
-  T get(std::string name, T defaultValue=T()) const
+  T get(const std::string& name, T defaultValue=T()) const
   {
     try
     {