Named Rules (#150)

Named rules allow users to have multiple of the same rules active at the same time, which allows them to enforce the same rule multiple times but with different options. Implements #113 Relates to #66
jorisroovers · Sep 25, 2020 · 7beb266 · 7beb266
1 parent abe45b1
commit 7beb266
Show file tree

Hide file tree

Showing 17 changed files with 418 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,7 +1,7 @@
 # Changelog #
 
 ## v0.14.0 (TBD) ##
-
+- [Named Rules](TODO) allow users to have multiple instances of the same rule active at the same time. This is useful when you want to enforce the same rule multiple times but with different options.
 - [User-defined Configuration Rules](TODO) allow users to dynamically change gitlint's configuration and/or the commit *before* any other rules are applied.
 - Users can now use `self.log.debug("my message")` for debugging purposes in their user-defined rules
 - Breaking: User-defined rule id's can no longer start with 'I', as those are reserved for built-in gitlint ignore rules.

diff --git a/docs/index.md b/docs/index.md
@@ -339,16 +339,76 @@ If you just want to ignore certain lines in a commit, you can do that using the
 regex=^Co-Authored-By
 ```
 
+!!! warning
+
+    When ignoring specific lines, gitlint will no longer be aware of them while applying other rules.
+    This can sometimes be confusing for end-users, especially as line numbers of violations will typically no longer
+    match line numbers in the original commit message. Make sure to educate your users accordingly.
+
 !!! note
 
     If you want to implement more complex ignore rules according to your own logic, you can do so using [user-defined
     configuration rules](user_defined_rules.md#configuration-rules).
 
+## Named Rules
+
+Introduced in gitlint v0.14.0
+
+Named rules allow you to have multiple of the same rules active at the same time, which allows you to
+enforce the same rule multiple times but with different options. Named rules are so-called because they require an
+additional unique identifier (i.e. the rule *name*) during configuration.
+
 !!! warning
 
-    When ignoring specific lines, gitlint will no longer be aware of them while applying other rules.
-    This can sometimes be confusing for end-users, especially as line numbers of violations will typically no longer
-    match line numbers in the original commit message. Make sure to educate your users accordingly.
+    Named rules is an advanced topic. It's easy to make mistakes by defining conflicting instances of the same rule.
+    For example, by defining 2 `body-max-line-length` rules with different `line-length` options, you obviously create
+    a conflicting situation. Gitlint does not do any resolution of such conflicts, it's up to you to make sure
+    any configuration is non-conflicting. So caution advised!
+
+Defining a named rule is easy, for example using your `.gitlint` file:
+
+```ini
+# By adding the following section, you will add a second instance of the
+# title-must-not-contain-word (T5) rule (in addition to the one that is enabled
+# by default) with the name 'extra-words'.
+[title-must-not-contain-word:extra-words]
+words=foo,bar
+
+# So the generic form is
+# [<rule-id-or-name>:<your-chosen-name>]
+# Another example, referencing the rule type by id
+[T5:more-words]
+words=hur,dur
+
+# You can add as many additional rules and you can name them whatever you want
+# The only requirement is that names cannot contain whitespace or colons (:)
+[title-must-not-contain-word:This-Can_Be*Whatever$YouWant]
+words=wonderwoman,batman,power ranger
+```
+
+When executing gitlint, you will see the violations from the default `title-must-not-contain-word (T5)` rule, as well as
+the violations caused by the additional Named Rules.
+
+```sh
+$ gitlint 
+1: T5 Title contains the word 'WIP' (case-insensitive): "WIP: foo wonderwoman hur bar"
+1: T5:This-Can_Be*Whatever$YouWant Title contains the word 'wonderwoman' (case-insensitive): "WIP: foo wonderwoman hur bar"
+1: T5:extra-words Title contains the word 'foo' (case-insensitive): "WIP: foo wonderwoman hur bar"
+1: T5:extra-words Title contains the word 'bar' (case-insensitive): "WIP: foo wonderwoman hur bar"
+1: T5:more-words Title contains the word 'hur' (case-insensitive): "WIP: foo wonderwoman hur bar"
+```
+
+Named rules are further treated identical to all other rules in gitlint:
+
+- you can reference by their full name, when e.g. adding them to your `ignore` configuration
+```ini
+# .gitlint file example
+[general]
+ignore=T5:more-words,title-must-not-contain-word:extra-words
+```
+
+- you can use them to instantiate multiple of the same [user-defined rule](user_defined_rules.md)
+- you can configure them using [any of the ways you can configure regular gitlint rules](configuration.md)
 
 
 ## Exit codes

diff --git a/gitlint/config.py b/gitlint/config.py
@@ -367,7 +367,10 @@ def __ne__(self, other):
     def __len__(self):
         return len(self._rules)
 
-    def __str__(self):
+    def __repr__(self):
+        return self.__unicode__()  # pragma: no cover
+
+    def __unicode__(self):
         return_str = ""
         for rule in self._rules.values():
             return_str += u"  {0}: {1}\n".format(rule.id, rule.name)
@@ -391,13 +394,15 @@ class LintConfigBuilder(object):
     normalized, validated and build. Example usage can be found in gitlint.cli.
     """
 
+    RULE_QUALIFIER_SYMBOL = ":"
+
     def __init__(self):
-        self._config_blueprint = {}
+        self._config_blueprint = OrderedDict()
         self._config_path = None
 
     def set_option(self, section, option_name, option_value):
         if section not in self._config_blueprint:
-            self._config_blueprint[section] = {}
+            self._config_blueprint[section] = OrderedDict()
         self._config_blueprint[section][option_name] = option_value
 
     def set_config_from_commit(self, commit):
@@ -444,10 +449,43 @@ def set_from_config_file(self, filename):
         except ConfigParserError as e:
             raise LintConfigError(ustr(e))
 
+    def _add_named_rule(self, config, qualified_rule_name):
+        """ Adds a Named Rule to a given LintConfig object.
+            IMPORTANT: This method does *NOT* overwrite existing Named Rules with the same canonical id.
+        """
+
+        # Split up named rule in its parts: the name/id that specifies the parent rule,
+        # And the name of the rule instance itself
+        rule_name_parts = qualified_rule_name.split(self.RULE_QUALIFIER_SYMBOL, 1)
+        rule_name = rule_name_parts[1].strip()
+        parent_rule_specifier = rule_name_parts[0].strip()
+
+        # assert that the rule name is valid:
+        # - not empty
+        # - no whitespace or colons
+        if rule_name == "" or bool(re.search("\\s|:", rule_name, re.UNICODE)):
+            msg = u"The rule-name part in '{0}' cannot contain whitespace, colons or be empty"
+            raise LintConfigError(msg.format(qualified_rule_name))
+
+        # find parent rule
+        parent_rule = config.rules.find_rule(parent_rule_specifier)
+        if not parent_rule:
+            msg = u"No such rule '{0}' (named rule: '{1}')"
+            raise LintConfigError(msg.format(parent_rule_specifier, qualified_rule_name))
+
+        # Determine canonical id and name by recombining the parent id/name and instance name parts.
+        canonical_id = parent_rule.__class__.id + self.RULE_QUALIFIER_SYMBOL + rule_name
+        canonical_name = parent_rule.__class__.name + self.RULE_QUALIFIER_SYMBOL + rule_name
+
+        # Add the rule to the collection of rules if it's not there already
+        if not config.rules.find_rule(canonical_id):
+            config.rules.add_rule(parent_rule.__class__, canonical_id, {'is_named': True, 'name': canonical_name})
+
+        return canonical_id
+
     def build(self, config=None):
         """ Build a real LintConfig object by normalizing and validating the options that were previously set on this
         factory. """
-
         # If we are passed a config object, then rebuild that object instead of building a new lintconfig object from
         # scratch
         if not config:
@@ -465,6 +503,12 @@ def build(self, config=None):
             for option_name, option_value in section_dict.items():
                 # Skip over the general section, as we've already done that above
                 if section_name != "general":
+
+                    # If the section name contains a colon (:), then this section is defining a Named Rule
+                    # Which means we need to instantiate that Named Rule in the config.
+                    if self.RULE_QUALIFIER_SYMBOL in section_name:
+                        section_name = self._add_named_rule(config, section_name)
+
                     config.set_rule_option(section_name, option_name, option_value)
 
         return config

diff --git a/gitlint/tests/cli/test_cli.py b/gitlint/tests/cli/test_cli.py
@@ -529,3 +529,18 @@ def test_no_commits_in_range(self, sh, _):
 
         self.assert_log_contains(u"DEBUG: gitlint.cli No commits in range \"master...HEAD\"")
         self.assertEqual(result.exit_code, 0)
+
+    @patch('gitlint.cli.get_stdin_data', return_value=u"WIP: tëst tïtle")
+    def test_named_rules(self, _):
+        with patch('gitlint.display.stderr', new=StringIO()) as stderr:
+            config_path = self.get_sample_path(os.path.join("config", "named-rules"))
+            result = self.cli.invoke(cli.cli, ["--config", config_path, "--debug"])
+            self.assertEqual(result.output, "")
+            self.assertEqual(stderr.getvalue(), self.get_expected("test_cli/test_named_rules_1"))
+            self.assertEqual(result.exit_code, 4)
+
+            # Assert debug logs are correct
+            expected_kwargs = self.get_system_info_dict()
+            expected_kwargs.update({'config_path': config_path})
+            expected_logs = self.get_expected('test_cli/test_named_rules_2', expected_kwargs)
+            self.assert_logged(expected_logs)
diff --git a/gitlint/tests/config/test_config_builder.py b/gitlint/tests/config/test_config_builder.py
@@ -1,9 +1,12 @@
 # -*- coding: utf-8 -*-
+import copy
 
 from gitlint.tests.base import BaseTestCase
 
 from gitlint.config import LintConfig, LintConfigBuilder, LintConfigError
 
+from gitlint import rules
+
 
 class LintConfigBuilderTests(BaseTestCase):
     def test_set_option(self):
@@ -202,3 +205,60 @@ def test_clone(self):
         # Modify the original and make sure we're not modifying the clone (i.e. check that the copy is a deep copy)
         config_builder.set_option('title-max-length', 'line-length', 120)
         self.assertDictEqual(cloned_builder._config_blueprint, expected)
+
+    def test_named_rules(self):
+        # Store a copy of the default rules from the config, so we can reference it later
+        config_builder = LintConfigBuilder()
+        config = config_builder.build()
+        default_rules = copy.deepcopy(config.rules)
+        self.assertEqual(default_rules, config.rules)  # deepcopy should be equal
+
+        # Add a named rule by setting an option in the config builder that follows the named rule pattern
+        # Assert that whitespace in the rule name is stripped
+        rule_qualifiers = [u'T7:my-extra-rüle', u' T7 :   my-extra-rüle  ', u'\tT7:\tmy-extra-rüle\t',
+                           u'T7:\t\n  \tmy-extra-rüle\t\n\n', u"title-match-regex:my-extra-rüle"]
+        for rule_qualifier in rule_qualifiers:
+            config_builder = LintConfigBuilder()
+            config_builder.set_option(rule_qualifier, 'regex', u"föo")
+
+            expected_rules = copy.deepcopy(default_rules)
+            my_rule = rules.TitleRegexMatches({'regex': u"föo"})
+            my_rule.id = rules.TitleRegexMatches.id + u":my-extra-rüle"
+            my_rule.name = rules.TitleRegexMatches.name + u":my-extra-rüle"
+            expected_rules._rules[u'T7:my-extra-rüle'] = my_rule
+            self.assertEqual(config_builder.build().rules, expected_rules)
+
+            # assert that changing an option on the newly added rule is passed correctly to the RuleCollection
+            # we try this with all different rule qualifiers to ensure they all are normalized and map
+            # to the same rule
+            for other_rule_qualifier in rule_qualifiers:
+                cb = config_builder.clone()
+                cb.set_option(other_rule_qualifier, 'regex', other_rule_qualifier + u"bōr")
+                # before setting the expected rule option value correctly, the RuleCollection should be different
+                self.assertNotEqual(cb.build().rules, expected_rules)
+                # after setting the option on the expected rule, it should be equal
+                my_rule.options['regex'].set(other_rule_qualifier + u"bōr")
+                self.assertEqual(cb.build().rules, expected_rules)
+                my_rule.options['regex'].set(u"wrong")
+
+    def test_named_rules_negative(self):
+        # T7 = title-match-regex
+        # Invalid rule name
+        for invalid_name in ["", " ", "    ", "\t", "\n", u"å b", u"å:b", u"åb:", u":åb"]:
+            config_builder = LintConfigBuilder()
+            config_builder.set_option(u"T7:{0}".format(invalid_name), 'regex', u"tëst")
+            expected_msg = u"The rule-name part in 'T7:{0}' cannot contain whitespace, colons or be empty"
+            with self.assertRaisesMessage(LintConfigError, expected_msg.format(invalid_name)):
+                config_builder.build()
+
+        # Invalid parent rule name
+        config_builder = LintConfigBuilder()
+        config_builder.set_option(u"Ž123:foöbar", u"fåke-option", u"fåke-value")
+        with self.assertRaisesMessage(LintConfigError, u"No such rule 'Ž123' (named rule: 'Ž123:foöbar')"):
+            config_builder.build()
+
+        # Invalid option name (this is the same as with regular rules)
+        config_builder = LintConfigBuilder()
+        config_builder.set_option(u"T7:foöbar", u"blå", u"my-rëgex")
+        with self.assertRaisesMessage(LintConfigError, u"Rule 'T7:foöbar' has no option 'blå'"):
+            config_builder.build()
diff --git a/gitlint/tests/expected/test_cli/test_named_rules_1 b/gitlint/tests/expected/test_cli/test_named_rules_1
@@ -0,0 +1,4 @@
+1: T5 Title contains the word 'WIP' (case-insensitive): "WIP: tëst tïtle"
+1: T5:even-more-wörds Title contains the word 'tïtle' (case-insensitive): "WIP: tëst tïtle"
+1: T5:extra-wörds Title contains the word 'tëst' (case-insensitive): "WIP: tëst tïtle"
+3: B6 Body message is missing
diff --git a/gitlint/tests/expected/test_cli/test_named_rules_2 b/gitlint/tests/expected/test_cli/test_named_rules_2
@@ -0,0 +1,80 @@
+DEBUG: gitlint.cli To report issues, please visit https://github.com/jorisroovers/gitlint/issues
+DEBUG: gitlint.cli Platform: {platform}
+DEBUG: gitlint.cli Python version: {python_version}
+DEBUG: gitlint.cli Git version: git version 1.2.3
+DEBUG: gitlint.cli Gitlint version: {gitlint_version}
+DEBUG: gitlint.cli GITLINT_USE_SH_LIB: {GITLINT_USE_SH_LIB}
+DEBUG: gitlint.cli DEFAULT_ENCODING: {DEFAULT_ENCODING}
+DEBUG: gitlint.cli Configuration
+config-path: {config_path}
+[GENERAL]
+extra-path: None
+contrib: []
+ignore: 
+ignore-merge-commits: True
+ignore-fixup-commits: True
+ignore-squash-commits: True
+ignore-revert-commits: True
+ignore-stdin: False
+staged: False
+verbosity: 3
+debug: True
+target: {target}
+[RULES]
+  I1: ignore-by-title
+     ignore=all
+     regex=None
+  I2: ignore-by-body
+     ignore=all
+     regex=None
+  I3: ignore-body-lines
+     regex=None
+  T1: title-max-length
+     line-length=72
+  T2: title-trailing-whitespace
+  T6: title-leading-whitespace
+  T3: title-trailing-punctuation
+  T4: title-hard-tab
+  T5: title-must-not-contain-word
+     words=WIP,bögus
+  T7: title-match-regex
+     regex=.*
+  B1: body-max-line-length
+     line-length=80
+  B5: body-min-length
+     min-length=20
+  B6: body-is-missing
+     ignore-merge-commits=True
+  B2: body-trailing-whitespace
+  B3: body-hard-tab
+  B4: body-first-line-empty
+  B7: body-changed-file-mention
+     files=
+  B8: body-match-regex
+     regex=None
+  M1: author-valid-email
+     regex=[^@ ]+@[^@ ]+\.[^@ ]+
+  T5:extra-wörds: title-must-not-contain-word:extra-wörds
+     words=hür,tëst
+  T5:even-more-wörds: title-must-not-contain-word:even-more-wörds
+     words=hür,tïtle
+
+DEBUG: gitlint.cli Stdin data: 'WIP: tëst tïtle'
+DEBUG: gitlint.cli Stdin detected and not ignored. Using as input.
+DEBUG: gitlint.git ('config', '--get', 'core.commentchar')
+DEBUG: gitlint.cli Linting 1 commit(s)
+DEBUG: gitlint.lint Linting commit [SHA UNKNOWN]
+DEBUG: gitlint.lint Commit Object
+--- Commit Message ----
+WIP: tëst tïtle
+--- Meta info ---------
+Author: None <None>
+Date:   None
+is-merge-commit:  False
+is-fixup-commit:  False
+is-squash-commit: False
+is-revert-commit: False
+Branches: []
+Changed Files: []
+-----------------------
+DEBUG: gitlint.cli Exit Code = 4
diff --git a/gitlint/tests/samples/config/named-rules b/gitlint/tests/samples/config/named-rules
@@ -0,0 +1,8 @@
+[title-must-not-contain-word]
+words=WIP,bögus
+
+[title-must-not-contain-word:extra-wörds]
+words=hür,tëst
+
+[T5:even-more-wörds]
+words=hür,tïtle