Adapt to Contour's best code quality & maintenance practice.

Signed-off-by: Christian Parpart <[email protected]>

Author: christianparpart

.github/workflows/build.yml

@@ -410,6 +410,8 @@ jobs:
         run: cmake --build build/ -- -j3
       - name: "test: crispy"
         run: ./build/src/crispy/crispy_test
+      - name: "test: regex_dfa"
+        run: ./build/src/regex_dfa/regex_dfa_test
       - name: "test: vtparser"
         run: ./build/src/vtparser/vtparser_test
       - name: "test: libterminal"
@@ -421,6 +423,7 @@ jobs:
           name: contour-ubuntu2204-tests
           path: |
             build/src/crispy/crispy_test
+            build/src/regex_dfa/regex_dfa_test
             build/src/vtparser/vtparser_test
             build/src/vtbackend/vtbackend_test
             build/src/vtbackend/bench-headless

src/regex_dfa/Alphabet.cpp

@@ -32,10 +32,10 @@ namespace regex_dfa
 
 void Alphabet::insert(Symbol ch)
 {
-    if (alphabet_.find(ch) == alphabet_.end())
+    if (_alphabet.find(ch) == _alphabet.end())
     {
         DEBUG("Alphabet: insert '{:}'", prettySymbol(ch));
-        alphabet_.insert(ch);
+        _alphabet.insert(ch);
     }
 }
 
@@ -45,7 +45,7 @@ string Alphabet::to_string() const
 
     sstr << '{';
 
-    for (Symbol c: alphabet_)
+    for (Symbol c: _alphabet)
         sstr << prettySymbol(c);
 
     sstr << '}';

src/regex_dfa/Alphabet.h

@@ -25,17 +25,17 @@ class Alphabet
     using set_type = std::set<Symbol>;
     using iterator = set_type::iterator;
 
-    size_t size() const noexcept { return alphabet_.size(); }
+    [[nodiscard]] size_t size() const noexcept { return _alphabet.size(); }
 
     void insert(Symbol ch);
 
-    std::string to_string() const;
+    [[nodiscard]] std::string to_string() const;
 
-    const iterator begin() const { return alphabet_.begin(); }
-    const iterator end() const { return alphabet_.end(); }
+    [[nodiscard]] iterator begin() const { return _alphabet.begin(); }
+    [[nodiscard]] iterator end() const { return _alphabet.end(); }
 
   private:
-    set_type alphabet_;
+    set_type _alphabet;
 };
 
 } // namespace regex_dfa

src/regex_dfa/CMakeLists.txt

@@ -21,7 +21,9 @@ target_include_directories(regex_dfa PUBLIC ${PROJECT_SOURCE_DIR}/src ${CMAKE_SO
 target_link_libraries(regex_dfa PUBLIC fmt::fmt-header-only)
 
 # ----------------------------------------------------------------------------
-if(TESTS)
+option(REGEX_DFA_TESTING "Enables building of unittests for regex_dfa library [default: ON]" ON)
+if(REGEX_DFA_TESTING)
+    enable_testing()
     add_executable(regex_dfa_test
         regex_dfa_test.cpp
         DFABuilder_test.cpp
@@ -33,9 +35,9 @@ if(TESTS)
         State_test.cpp
         Symbols_test.cpp
         util/iterator_test.cpp
-        util/testing.cpp
     )
 
     target_link_libraries(regex_dfa_test PUBLIC regex_dfa)
+    target_link_libraries(regex_dfa_test PUBLIC Catch2::Catch2)
     target_link_libraries(regex_dfa_test PUBLIC fmt::fmt-header-only)
-endif(TESTS)
+endif(REGEX_DFA_TESTING)

src/regex_dfa/CharStream.h

@@ -27,41 +27,41 @@ class CharStream
 class StringStream: public CharStream
 {
   public:
-    explicit StringStream(std::string&& s): source_ { std::move(s) } {}
+    explicit StringStream(std::string&& s): _source { std::move(s) } {}
 
-    [[nodiscard]] bool isEof() const noexcept override { return pos_ >= source_.size(); }
-    char get() override { return source_[pos_++]; }
-    void rollback(int count) override { pos_ -= count; }
-    void rewind() override { pos_ = 0; }
+    [[nodiscard]] bool isEof() const noexcept override { return _pos >= _source.size(); }
+    char get() override { return _source[_pos++]; }
+    void rollback(int count) override { _pos -= count; }
+    void rewind() override { _pos = 0; }
 
   private:
-    std::string source_;
-    size_t pos_ = 0;
+    std::string _source;
+    size_t _pos = 0;
 };
 
 class StandardStream: public CharStream
 {
   public:
     explicit StandardStream(std::istream* source);
 
-    [[nodiscard]] bool isEof() const noexcept override { return !source_->good(); }
-    char get() override { return static_cast<char>(source_->get()); }
+    [[nodiscard]] bool isEof() const noexcept override { return !_source->good(); }
+    char get() override { return static_cast<char>(_source->get()); }
 
     void rollback(int count) override
     {
-        source_->clear();
-        source_->seekg(-count, std::ios::cur);
+        _source->clear();
+        _source->seekg(-count, std::ios::cur);
     }
 
     void rewind() override
     {
-        source_->clear();
-        source_->seekg(initialOffset_, std::ios::beg);
+        _source->clear();
+        _source->seekg(_initialOffset, std::ios::beg);
     }
 
   private:
-    std::istream* source_;
-    std::streamoff initialOffset_;
+    std::istream* _source;
+    std::streamoff _initialOffset;
 };
 
 } // namespace regex_dfa

src/regex_dfa/Compiler.cpp

@@ -134,7 +134,7 @@ DFA Compiler::compileMinimalDFA()
 
 LexerDef Compiler::compile()
 {
-    return generateTables(compileMinimalDFA(), containsBeginOfLine_, move(names_));
+    return generateTables(compileMinimalDFA(), containsBeginOfLine_, std::move(names_));
 }
 
 LexerDef Compiler::compileMulti(OvershadowMap* overshadows)

src/regex_dfa/Compiler.h

@@ -48,9 +48,9 @@ class Compiler
      */
     void declareAll(RuleList rules);
 
-    const RuleList& rules() const noexcept { return rules_; }
-    const TagNameMap& names() const noexcept { return names_; }
-    size_t size() const;
+    [[nodiscard]] const RuleList& rules() const noexcept { return rules_; }
+    [[nodiscard]] const TagNameMap& names() const noexcept { return names_; }
+    [[nodiscard]] size_t size() const;
 
     /**
      * Compiles all previousely parsed rules into a DFA.
@@ -84,9 +84,9 @@ class Compiler
     static LexerDef generateTables(const DFA& dfa, bool requiresBeginOfLine, const TagNameMap& names);
     static LexerDef generateTables(const MultiDFA& dfa, bool requiresBeginOfLine, const TagNameMap& names);
 
-    const std::map<std::string, NFA>& automata() const { return fa_; }
+    [[nodiscard]] const std::map<std::string, NFA>& automata() const { return fa_; }
 
-    bool containsBeginOfLine() const noexcept { return containsBeginOfLine_; }
+    [[nodiscard]] bool containsBeginOfLine() const noexcept { return containsBeginOfLine_; }
 
   private:
     /**

src/regex_dfa/DFA.cpp

@@ -37,7 +37,7 @@ Alphabet DFA::alphabet() const
 {
     Alphabet alphabet;
     for (const State& state: states_)
-        for (const pair<Symbol, StateId>& t: state.transitions)
+        for (pair<Symbol, StateId> const t: state.transitions)
             alphabet.insert(t.first);
 
     return alphabet;
@@ -118,12 +118,12 @@ void DFA::prepareStateIds(StateId baseId, StateId q0)
     AcceptMap remapped;
     for (auto& a: acceptTags_)
         remapped[transformId(a.first)] = a.second;
-    acceptTags_ = move(remapped);
+    acceptTags_ = std::move(remapped);
 
     BacktrackingMap backtracking;
     for (const auto& bt: backtrackStates_)
         backtracking[transformId(bt.first)] = transformId(bt.second);
-    backtrackStates_ = move(backtracking);
+    backtrackStates_ = std::move(backtracking);
 
     initialState_ = q0;
 }

src/regex_dfa/DFA.h

@@ -57,16 +57,16 @@ class DFA
     }
 
     //! Retrieves the alphabet of this finite automaton.
-    Alphabet alphabet() const;
+    [[nodiscard]] Alphabet alphabet() const;
 
     //! Retrieves the initial state.
-    StateId initialState() const { return initialState_; }
+    [[nodiscard]] StateId initialState() const { return initialState_; }
 
     //! Retrieves the list of available states.
-    const StateVec& states() const { return states_; }
-    StateVec& states() { return states_; }
+    [[nodiscard]] const StateVec& states() const { return states_; }
+    [[nodiscard]] StateVec& states() { return states_; }
 
-    StateIdVec stateIds() const
+    [[nodiscard]] StateIdVec stateIds() const
     {
         StateIdVec v;
         v.reserve(states_.size());
@@ -76,7 +76,7 @@ class DFA
     }
 
     //! Retrieves the list of accepting states.
-    std::vector<StateId> acceptStates() const;
+    [[nodiscard]] std::vector<StateId> acceptStates() const;
 
     /**
      * Traverses all states and edges in this NFA and calls @p visitor for each state & edge.
@@ -89,39 +89,39 @@ class DFA
 
     void setInitialState(StateId state);
 
-    const TransitionMap& stateTransitions(StateId id) const
+    [[nodiscard]] const TransitionMap& stateTransitions(StateId id) const
     {
         return states_[static_cast<size_t>(id)].transitions;
     }
 
     // {{{ backtracking (for lookahead)
     void setBacktrack(StateId from, StateId to) { backtrackStates_[from] = to; }
 
-    std::optional<StateId> backtrack(StateId acceptState) const
+    [[nodiscard]] std::optional<StateId> backtrack(StateId acceptState) const
     {
         if (auto i = backtrackStates_.find(acceptState); i != backtrackStates_.end())
             return i->second;
 
         return std::nullopt;
     }
 
-    const BacktrackingMap& backtracking() const noexcept { return backtrackStates_; }
+    [[nodiscard]] const BacktrackingMap& backtracking() const noexcept { return backtrackStates_; }
     // }}}
 
     //! Flags given state as accepting-state with given Tag @p acceptTag.
     void setAccept(StateId state, Tag acceptTag) { acceptTags_[state] = acceptTag; }
 
-    bool isAccepting(StateId s) const { return acceptTags_.find(s) != acceptTags_.end(); }
+    [[nodiscard]] bool isAccepting(StateId s) const { return acceptTags_.find(s) != acceptTags_.end(); }
 
-    std::optional<Tag> acceptTag(StateId s) const
+    [[nodiscard]] std::optional<Tag> acceptTag(StateId s) const
     {
         if (auto i = acceptTags_.find(s); i != acceptTags_.end())
             return i->second;
 
         return std::nullopt;
     }
 
-    std::optional<StateId> delta(StateId state, Symbol symbol) const
+    [[nodiscard]] std::optional<StateId> delta(StateId state, Symbol symbol) const
     {
         const auto& T = states_[state].transitions;
         if (auto i = T.find(symbol); i != T.end())
@@ -133,7 +133,7 @@ class DFA
     void setTransition(StateId from, Symbol symbol, StateId to);
     void removeTransition(StateId from, Symbol symbol);
 
-    StateIdVec nonAcceptStates() const
+    [[nodiscard]] StateIdVec nonAcceptStates() const
     {
         StateIdVec result;
         result.reserve(
@@ -146,9 +146,9 @@ class DFA
         return result;
     }
 
-    bool isAcceptor(Tag t) const
+    [[nodiscard]] bool isAcceptor(Tag t) const
     {
-        for (const std::pair<StateId, Tag>& p: acceptTags_)
+        for (std::pair<StateId, Tag> p: acceptTags_)
             if (p.second == t)
                 return true;
 

src/regex_dfa/DFABuilder.cpp

@@ -166,7 +166,7 @@ DFA DFABuilder::constructDFA(const vector<StateIdVec>& Q,
 
     // observe mapping from q_i to d_i
     for (auto const& [q_i, branch]: T.transitions)
-        for (auto const [c, t_i]: branch)
+        for (auto && [c, t_i]: branch)
             dfa.setTransition(q_i, c, t_i);
 
     // q_0 becomes d_0 (initial state)

src/regex_dfa/DFABuilder.h

@@ -32,19 +32,20 @@ class DFABuilder
      * @param overshadows if not nullptr, it will be used to store semantic information about
      *                    which rule tags have been overshadowed by which.
      */
-    DFA construct(OvershadowMap* overshadows = nullptr);
+    [[nodiscard]] DFA construct(OvershadowMap* overshadows = nullptr);
 
   private:
     struct TransitionTable;
 
-    DFA constructDFA(const std::vector<StateIdVec>& Q,
-                     const TransitionTable& T,
-                     OvershadowMap* overshadows) const;
+    [[nodiscard]] DFA constructDFA(const std::vector<StateIdVec>& Q,
+                                   const TransitionTable& T,
+                                   OvershadowMap* overshadows) const;
 
     /**
      * Finds @p t in @p Q and returns its offset (aka configuration number) or -1 if not found.
      */
-    static std::optional<StateId> configurationNumber(const std::vector<StateIdVec>& Q, const StateIdVec& t);
+    [[nodiscard]] static std::optional<StateId> configurationNumber(const std::vector<StateIdVec>& Q,
+                                                                    const StateIdVec& t);
 
     /**
      * Determines the tag to use for the deterministic state representing @p q from non-deterministic FA @p
@@ -54,7 +55,7 @@ class DFABuilder
      *
      * @returns the determined tag or std::nullopt if none
      */
-    std::optional<Tag> determineTag(const StateIdVec& q, std::map<Tag, Tag>* overshadows) const;
+    [[nodiscard]] std::optional<Tag> determineTag(const StateIdVec& q, std::map<Tag, Tag>* overshadows) const;
 
   private:
     const NFA nfa_;

src/regex_dfa/DFABuilder_test.cpp

@@ -10,14 +10,14 @@
 #include <regex_dfa/DFABuilder.h>
 #include <regex_dfa/MultiDFA.h>
 
+#include <catch2/catch.hpp>
+
 #include <memory>
 #include <sstream>
 
-#include <klex/util/testing.h>
-
 using namespace regex_dfa;
 
-TEST(regex_DFABuilder, shadowing)
+TEST_CASE("regex_DFABuilder.shadowing")
 {
     Compiler cc;
     cc.parse(std::make_unique<std::stringstream>(R"(
@@ -27,7 +27,7 @@ TEST(regex_DFABuilder, shadowing)
     // rule 2 is overshadowed by rule 1
     Compiler::OvershadowMap overshadows;
     DFA dfa = cc.compileDFA(&overshadows);
-    ASSERT_EQ(1, overshadows.size());
-    EXPECT_EQ(2, overshadows[0].first);  // overshadowee
-    EXPECT_EQ(1, overshadows[0].second); // overshadower
+    REQUIRE(1 == overshadows.size());
+    CHECK(2 == overshadows[0].first);  // overshadowee
+    CHECK(1 == overshadows[0].second); // overshadower
 }