diff options
| -rw-r--r-- | docs/user_defined_rules.md | 44 | ||||
| -rw-r--r-- | examples/gitlint | 3 | ||||
| -rw-r--r-- | gitlint/files/gitlint | 6 | ||||
| -rw-r--r-- | gitlint/options.py | 3 | ||||
| -rw-r--r-- | gitlint/tests/samples/user_rules/bogus-file.txt | 2 | ||||
| -rw-r--r-- | gitlint/tests/samples/user_rules/import_exception/invalid_python.py | 3 | ||||
| -rw-r--r-- | gitlint/tests/samples/user_rules/my_commit_rules.foo | 16 | ||||
| -rw-r--r-- | gitlint/tests/samples/user_rules/my_commit_rules.py | 9 | ||||
| -rw-r--r-- | gitlint/tests/test_options.py | 11 | ||||
| -rw-r--r-- | gitlint/tests/test_user_rules.py | 83 | ||||
| -rw-r--r-- | gitlint/user_rules.py | 19 | ||||
| -rw-r--r-- | mkdocs.yml | 3 | ||||
| -rwxr-xr-x | run_tests.sh | 2 |
13 files changed, 194 insertions, 10 deletions
diff --git a/docs/user_defined_rules.md b/docs/user_defined_rules.md new file mode 100644 index 0000000..7b6d499 --- /dev/null +++ b/docs/user_defined_rules.md @@ -0,0 +1,44 @@ +# User Defined Rules +Gitlint versions 0.8.0 and above support the concept of User Defined rules: the ability for you +to write your own custom rules that are executed when gitlint is. + +This can be done using the ```extra-path``` general option, which can be set using the ```--extra-path``` +commandline flag, by adding it under the ```[general]``` section in your ```.gitlint``` file or using +one of the other ways to configure gitlint. For more details, please refer to the +[Configuration](configuration.md) page. + +If you want to check whether your rules are discovered, you can use the ```--debug``` flag: + +```bash +$ gitlint --debug +TODO: CONTINUE +``` + + +## TODO: +- Document extra_config parameter in the configuration section + +## Rule requirements + +As long as you stick with simple scenarios that are similar to the sample User Defined rules (see the ```examples``` directory), gitlint +should be able to discover and execute your custom rules. If you want to do something more exotic however, you might run into some issues. + +While the [rule finding source-code](https://github.com/jorisroovers/gitlint/blob/master/gitlint/user_rules.py) is the +ultimate source of truth, here are some of the requirements that gitlint enforces: + +### Extra path requirements ### +- The ```extra-path``` option must point to a **directory**, not a file +- The ```extra-path``` directory does **not** need to be a proper python package, i.e. it doesn't require an ```__init__.py``` file. +- Python files containing user rules must have a ```.py``` extension. Files with a different extension will be ignored. +- The ```extra-path``` will be searched non-recursively, i.e. all rule classes must be present at the top level ```extra-path``` directory. +- User rule classes must be defined in the modules that are part of ```extra-path```, rules that are imported from outside the ```extra-path``` will be ignored. + +### Rule class requirements ### + +- Rules *must* extend from ```LineRule``` or ```CommitRule``` +- Rule classes *must* have ```id``` and ```name``` string attributes. The ```options_spec``` is optional, but if set, it *must* be a list. +- Rule classes *must* have a ```validate``` method. In case of a ```CommitRule```, ```validate``` *must* take a single ```commit``` parameter. + In case of ```LineRule```, ```validate``` must take ```line``` and ```commit``` as first and second parameters. +- User Rule id's *cannot* be of the form ```R[0-9]+```, ```T[0-9]+```, ```B[0-9]+``` or ```M[0-9]+``` as these rule ids are reserved for gitlint itself. +- Rule *should* have a unique id as only one rule can exist with a given id. While gitlint does not enforce this, the rule that will + actually be chosen will be system specific. diff --git a/examples/gitlint b/examples/gitlint index aac5c0b..612b125 100644 --- a/examples/gitlint +++ b/examples/gitlint @@ -8,6 +8,9 @@ ignore-merge-commits=true # Enable debug mode (prints more output). Disabled by default debug = true +# Set the extra path where gitlint will search for user defined rules (uncomment to test) +# extra-path=examples/ + [title-max-length] line-length=20 diff --git a/gitlint/files/gitlint b/gitlint/files/gitlint index 1a27092..776418c 100644 --- a/gitlint/files/gitlint +++ b/gitlint/files/gitlint @@ -1,14 +1,16 @@ # All these sections are optional, edit this file as you like. # [general] # ignore=title-trailing-punctuation, T3 -# verbosity should be a value between 1 and 3, the commandline -v flags take precedence over -# this +# verbosity should be a value between 1 and 3, the commandline -v flags take precedence over this # verbosity = 2 # By default gitlint will ignore merge commits. Set to 'false' to disable. # ignore-merge-commits=true # Enable debug mode (prints more output). Disabled by default. # debug=true +# Set the extra path where gitlint will search for user defined rules +# extra-path=examples/ + # [title-max-length] # line-length=80 diff --git a/gitlint/options.py b/gitlint/options.py index 93e1042..194fc59 100644 --- a/gitlint/options.py +++ b/gitlint/options.py @@ -24,6 +24,9 @@ class RuleOption(object): def __repr__(self): return self.__str__() # pragma: no cover + def __eq__(self, other): + return self.name == other.name and self.description == other.description and self.value == other.value + class StrOption(RuleOption): def set(self, value): diff --git a/gitlint/tests/samples/user_rules/bogus-file.txt b/gitlint/tests/samples/user_rules/bogus-file.txt new file mode 100644 index 0000000..2a56650 --- /dev/null +++ b/gitlint/tests/samples/user_rules/bogus-file.txt @@ -0,0 +1,2 @@ +This is just a bogus file. +This file being here is part of the test: gitlint should ignore it.
\ No newline at end of file diff --git a/gitlint/tests/samples/user_rules/import_exception/invalid_python.py b/gitlint/tests/samples/user_rules/import_exception/invalid_python.py new file mode 100644 index 0000000..e75fed3 --- /dev/null +++ b/gitlint/tests/samples/user_rules/import_exception/invalid_python.py @@ -0,0 +1,3 @@ +# flake8: noqa +# This is invalid python code which will cause an import exception +class MyObject: diff --git a/gitlint/tests/samples/user_rules/my_commit_rules.foo b/gitlint/tests/samples/user_rules/my_commit_rules.foo new file mode 100644 index 0000000..605d704 --- /dev/null +++ b/gitlint/tests/samples/user_rules/my_commit_rules.foo @@ -0,0 +1,16 @@ +# This rule is ignored because it doesn't have a .py extension +from gitlint.rules import CommitRule, RuleViolation +from gitlint.options import IntOption + + +class MyUserCommitRule2(CommitRule): + name = "my-user-commit-rule2" + id = "TUC2" + options_spec = [IntOption('violation-count', 0, "Number of violations to return")] + + def validate(self, _commit): + violations = [] + for i in range(1, self.options['violation-count'].value + 1): + violations.append(RuleViolation(self.id, "Commit violation %d" % i, "Content %d" % i, i)) + + return violations diff --git a/gitlint/tests/samples/user_rules/my_commit_rules.py b/gitlint/tests/samples/user_rules/my_commit_rules.py index 3752dd5..6d6509e 100644 --- a/gitlint/tests/samples/user_rules/my_commit_rules.py +++ b/gitlint/tests/samples/user_rules/my_commit_rules.py @@ -13,3 +13,12 @@ class MyUserCommitRule(CommitRule): violations.append(RuleViolation(self.id, "Commit violation %d" % i, "Content %d" % i, i)) return violations + + +# The below code is present so that we can test that we actually ignore it + +def func_should_be_ignored(): + pass + + +global_variable_should_be_ignored = True diff --git a/gitlint/tests/test_options.py b/gitlint/tests/test_options.py index 4f9f59e..5b3fc9a 100644 --- a/gitlint/tests/test_options.py +++ b/gitlint/tests/test_options.py @@ -6,6 +6,17 @@ from gitlint.options import IntOption, BoolOption, StrOption, ListOption, Direct class RuleOptionTests(BaseTestCase): + def test_option_equals(self): + # 2 options are equal if their name, value and description match + option1 = IntOption("test-option", 123, "Test Description") + option2 = IntOption("test-option", 123, "Test Description") + self.assertEqual(option1, option2) + + # Not equal: name, description, value are different + self.assertNotEqual(option1, IntOption("test-option1", 123, "Test Description")) + self.assertNotEqual(option1, IntOption("test-option", 1234, "Test Description")) + self.assertNotEqual(option1, IntOption("test-option", 123, "Test Description2")) + def test_int_option(self): # normal behavior option = IntOption("test-name", 123, "Test Description") diff --git a/gitlint/tests/test_user_rules.py b/gitlint/tests/test_user_rules.py index e69de29..756b72b 100644 --- a/gitlint/tests/test_user_rules.py +++ b/gitlint/tests/test_user_rules.py @@ -0,0 +1,83 @@ +import sys + +from gitlint.tests.base import BaseTestCase +from gitlint.user_rules import find_rule_classes, assert_valid_rule_class, UserRuleError + +from gitlint import options, rules + + +class UserRuleTests(BaseTestCase): + def test_find_rule_classes(self): + # Let's find some user classes! + user_rule_path = self.get_sample_path("user_rules") + classes = find_rule_classes(user_rule_path) + + # Compare string representations because we can't import MyUserCommitRule here since samples/user_rules is not + # a proper python package + # Note that the following check effectively asserts that: + # - There is only 1 rule recognized and it is MyUserCommitRule + # - Other non-python files in the directory are ignored + # - Other members of the my_commit_rules module are ignored + # (such as func_should_be_ignored, global_variable_should_be_ignored) + # - Rules are loaded non-recursively (user_rules/import_exception directory is ignored) + self.assertEqual("[<class 'my_commit_rules.MyUserCommitRule'>]", str(classes)) + + # Assert that we added the new user_rules directory to the system path and modules + self.assertIn(user_rule_path, sys.path) + self.assertIn("my_commit_rules", sys.modules) + + # Do some basic asserts on our user rule + self.assertEqual(classes[0].id, "TUC1") + self.assertEqual(classes[0].name, "my-user-commit-rule") + expected_option = options.IntOption('violation-count', 0, "Number of violations to return") + self.assertListEqual(classes[0].options_spec, [expected_option]) + self.assertTrue(hasattr(classes[0], "validate")) + + # Test that we can instantiate the class and can execute run the validate method and that it returns the + # expected result + rule_class = classes[0]() + violations = rule_class.validate("false-commit-object (ignored)") + self.assertListEqual(violations, []) + + # Have it return a violation + rule_class.options['violation-count'].value = 1 + violations = rule_class.validate("false-commit-object (ignored)") + self.assertListEqual(violations, [rules.RuleViolation("TUC1", "Commit violation 1", "Content 1", 1)]) + + def test_empty_user_classes(self): + # Test that we don't find rules if we scan a different directory + user_rule_path = self.get_sample_path("config") + classes = find_rule_classes(user_rule_path) + self.assertListEqual(classes, []) + + # Importantly, ensure that the directory is not added to the syspath as this happens only when we actually + # find modules + self.assertNotIn(user_rule_path, sys.path) + + def test_failed_module_import(self): + # test importing a bogus module + user_rule_path = self.get_sample_path("user_rules/import_exception") + # We don't check the entire error message because that is different based on the python version and underlying + # operating system + expected_msg = "Error while importing extra-path module 'invalid_python'" + with self.assertRaisesRegexp(UserRuleError, expected_msg): + find_rule_classes(user_rule_path) + + def test_find_rule_classes_nonexisting_path(self): + # When searching an non-existing path, we expect an OSError. That's fine because this case will be caught by + # the CLI (you cannot specify a non-existing directory). What we do here is just assert that we indeed + # get an OSError (so we guard against regressions). + with self.assertRaisesRegexp(OSError, "No such file or directory"): + find_rule_classes("foo/bar") + + def test_assert_valid_rule_class(self): + class MyRuleClass(rules.Rule): + pass + + self.assertTrue(assert_valid_rule_class(MyRuleClass)) + + def test_assert_valid_rule_class_negative(self): + class MyNormalClass(object): + pass + + self.assertFalse(assert_valid_rule_class(MyNormalClass)) diff --git a/gitlint/user_rules.py b/gitlint/user_rules.py index 2d7865b..85065a2 100644 --- a/gitlint/user_rules.py +++ b/gitlint/user_rules.py @@ -7,6 +7,11 @@ import sys from gitlint import rules +class UserRuleError(Exception): + """ Error used to indicate that an error occurred while trying to load a user rule """ + pass + + def find_rule_classes(extra_path): """ Searches a given directory for rule classes. This is done by finding all python modules in the given directory, @@ -33,7 +38,10 @@ def find_rule_classes(extra_path): rule_classes = [] for module in modules: # Import the module - importlib.import_module(module) + try: + importlib.import_module(module) + except Exception as e: + raise UserRuleError("Error while importing extra-path module '{0}': {1}".format(module, str(e))) # Find all rule classes in the module. We do this my inspecting all members of the module and checking # 1) is it a class, if not, skip @@ -41,15 +49,14 @@ def find_rule_classes(extra_path): # 3) is it a subclass of rule rule_classes.extend([clazz for _, clazz in inspect.getmembers(sys.modules[module]) if - inspect.isclass(clazz) and - clazz.__module__ == module and - issubclass(clazz, rules.Rule) and + inspect.isclass(clazz) and # check isclass to ensure clazz.__module__ exists + clazz.__module__ == module and # ignore imported classes assert_valid_rule_class(clazz)]) return rule_classes -def assert_valid_rule_class(_clazz): +def assert_valid_rule_class(clazz): # TODO (joris.roovers): checks whether the rule class is valid # e.g.: it has an id and name and are of type string, default constructor, etc - return True + return issubclass(clazz, rules.Rule) @@ -4,8 +4,9 @@ site_url: http://jorisroovers.github.io/gitlint/ repo_url: https://github.com/jorisroovers/gitlint pages: - Home: index.md - - Rules: rules.md - Configuration: configuration.md + - Rules: rules.md + - User Defined Rules: user_defined_rules.md - Contributing: contributing.md markdown_extensions: [admonition] diff --git a/run_tests.sh b/run_tests.sh index 38b2908..7a8f306 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -47,7 +47,7 @@ run_pep8_check(){ # exclude settings files and virtualenvs FLAKE8_EXCLUDE="*settings.py,*.venv/*.py" echo "Running flake8..." - flake8 --ignore=$FLAKE8_IGNORE --max-line-length=120 --exclude=$FLAKE8_EXCLUDE gitlint qa + flake8 --ignore=$FLAKE8_IGNORE --max-line-length=120 --exclude=$FLAKE8_EXCLUDE gitlint qa examples } run_unit_tests(){ |