Support for fixup and squash commits (jorisroovers#43)

- Config code
- Git parsing code
- Linting skips
- Unit and Integration Tests
- Docs

This fixes jorisroovers#33.

docs/configuration.md

@@ -17,51 +17,63 @@ The block below shows a sample ```.gitlint``` file. Details about rule config op
 
 ```ini
 # All these sections are optional, edit this file as you like.
-[general]
-ignore=title-trailing-punctuation, T3
+# [general]
+# Ignore certain rules, you can reference them by their id or by their full name
+# ignore=title-trailing-punctuation, T3
+
 # verbosity should be a value between 1 and 3, the commandline -v flags take precedence over this
-verbosity = 2
+# verbosity = 2
+
 # By default gitlint will ignore merge commits. Set to 'false' to disable.
-ignore-merge-commits=true
+# ignore-merge-commits=true
+
+# By default gitlint will ignore fixup commits. Set to 'false' to disable.
+# ignore-fixup-commits=true
+
+# By default gitlint will ignore squash commits. Set to 'false' to disable.
+# ignore-squash-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
 # See http://jorisroovers.github.io/gitlint/user_defined_rules for details
 # extra-path=examples/
 
-[title-max-length]
-line-length=20
+# [title-max-length]
+# line-length=80
 
-[title-must-not-contain-word]
+# [title-must-not-contain-word]
 # Comma-separated list of words that should not occur in the title. Matching is case
 # insensitive. It's fine if the keyword occurs as part of a larger word (so "WIPING"
 # will not cause a violation, but "WIP: my title" will.
-words=wip,title
+# words=wip
 
-[title-match-regex]
+# [title-match-regex]
 # python like regex (https://docs.python.org/2/library/re.html) that the
 # commit-msg title must be matched to.
 # Note that the regex can contradict with other rules if not used correctly
 # (e.g. title-must-not-contain-word).
-regex=^US[0-9]*
+# regex=^US[0-9]*
 
-[B1]
+# [B1]
 # B1 = body-max-line-length
-line-length=30
+# line-length=120
 
-[body-min-length]
-min-length=5
+# [body-min-length]
+# min-length=5
 
-[body-is-missing]
+# [body-is-missing]
 # Whether to ignore this rule on merge commits (which typically only have a title)
 # default = True
-ignore-merge-commits=false
+# ignore-merge-commits=false
 
-[body-changed-file-mention]
+# [body-changed-file-mention]
 # List of files that need to be explicitly mentioned in the body when they are changed
 # This is useful for when developers often erroneously edit certain files or git submodules.
 # By specifying this rule, developers can only change the file when they explicitly reference
 # it in the commit message.
-files=gitlint/rules.py,README.md
+# files=gitlint/rules.py,README.md
 
 # [author-valid-email]
 # python like regex (https://docs.python.org/2/library/re.html) that the
@@ -77,7 +89,7 @@ You can also use one or more ```-c``` flags like so:
 ```
 $ gitlint -c general.verbosity=2 -c title-max-length.line-length=80 -c B1.line-length=100
 ```
-The generic config flag format is ```-c <rule>.<option>=<value>``` and supports all the same rules and options which 
+The generic config flag format is ```-c <rule>.<option>=<value>``` and supports all the same rules and options which
 you can also use in a ```.gitlint``` config file.
 
 # Commit specific config #
@@ -98,7 +110,7 @@ specific rules to be ignored as follows: ```gitlint-ignore: T1, body-hard-tab```
 # Configuration precedence #
 gitlint configuration is applied in the following order of precedence:
 
-1. Commit specific config (e.g.: ```gitlint-ignore: all``` in the commit message) 
+1. Commit specific config (e.g.: ```gitlint-ignore: all``` in the commit message)
 2. Commandline convenience flags (e.g.:  ```-vv```, ```--silent```, ```--ignore```)
 3. Commandline configuration flags (e.g.: ```-c title-max-length=123```)
 4. Configuration file (local ```.gitlint``` file, or file specified using ```-C```/```--config```)
@@ -109,11 +121,13 @@ The table below outlines configuration options that modify gitlint's overall beh
 using commandline flags or in ```general``` section in a ```.gitlint``` configuration file.
 
 Name                 | Default value |  gitlint version | commandline flag                      | Description
----------------------|---------------|------------------|---------------------------------------|-------------------------------------
-silent               | false         | >= 0.1           | ```--silent```                        | Enable silent mode (no output). Use [exit](index.md#exit-codes) code to determine result.
-verbosity            | 3             | >= 0.1           | ```--verbosity=3```                   | Amount of output gitlint will show when printing errors.
-ignore-merge-commits | true          | >= 0.7.0         |   Not available                       | Whether or not to ignore merge commits.
-ignore               | [] (=empty)   | >= 0.1           | ```--ignore=T1,body-min-length```     | Comma seperated list of rules to ignore (by name or id)
-debug                | false         | >= 0.7.1         |  ```--debug```                        | Enable debugging output
-target               | (empty)       | >= 0.8.0         |  ```---target=/home/joe/myrepo/   ``` | Target git repository gitlint should be linting against.
-extra-path           | (empty)       | >= 0.8.0         |  ```---extra-path=/home/joe/rules/``` | Path where gitlint looks for [user-defined rules](user_defined_rules.md).
+----------------------|---------------|------------------|---------------------------------------|-------------------------------------
+silent                | false         | >= 0.1           | ```--silent```                        | Enable silent mode (no output). Use [exit](index.md#exit-codes) code to determine result.
+verbosity             | 3             | >= 0.1           | ```--verbosity=3```                   | Amount of output gitlint will show when printing errors.
+ignore-merge-commits  | true          | >= 0.7.0         |   Not available                       | Whether or not to ignore merge commits.
+ignore-fixup-commits  | true          | >= 0.9.0         |   Not available                       | Whether or not to ignore [fixup](https://git-scm.com/docs/git-commit#git-commit---fixupltcommitgt) commits.
+ignore-squash-commits | true          | >= 0.9.0         |   Not available                       | Whether or not to ignore [squash](https://git-scm.com/docs/git-commit#git-commit---squashltcommitgt) commits.
+ignore                | [] (=empty)   | >= 0.1           | ```--ignore=T1,body-min-length```     | Comma seperated list of rules to ignore (by name or id)
+debug                 | false         | >= 0.7.1         |  ```--debug```                        | Enable debugging output
+target                | (empty)       | >= 0.8.0         |  ```---target=/home/joe/myrepo/   ``` | Target git repository gitlint should be linting against.
+extra-path            | (empty)       | >= 0.8.0         |  ```---extra-path=/home/joe/rules/``` | Path where gitlint looks for [user-defined rules](user_defined_rules.md).

docs/contributing.md

@@ -1,5 +1,6 @@
 We'd love for you to [contribute to gitlint](https://github.com/jorisroovers/gitlint).
-Just create an issue or open a pull request and we'll get right on it!
+Just create an issue or open a pull request. Sometimes it takes a while for me to get back to you
+(this is a hobby project), but rest assured that we read your message and appreciate your interest!
 We maintain a [wishlist on our wiki](https://github.com/jorisroovers/gitlint/wiki/Wishlist),
 but we're obviously open to any suggestions!
 

docs/index.md

@@ -192,19 +192,28 @@ done
     lint a large set of commits. Always use ```--commits``` if you can to avoid this performance penalty.
 
 
-## Merge commits ##
-_Introduced in gitlint v0.7.0_
+## Merge, fixup and squash commits ##
+_Introduced in gitlint v0.7.0 (merge commits) and gilint v0.9.0 (fixup, squash)_
 
-Gitlint ignores merge commits by default. The rationale behind this is that in many cases
-merge commits are not created by users themselves but by tools such as github,
-[gerrit](https://code.google.com/p/gerrit/) and others. These tools often generate merge commit messages that
-violate gitlint's set of rules and it's not always convenient or desired to change those.
+**Gitlint ignores merge, fixup and squash commits by default.**
 
-In case you *do* want to lint merge commit messages, you can disable this behavior by setting the
-general ```ignore-merge-commits``` option to ```false```
+For merge commits, the rationale for ignoring them is
+that in many cases merge commits are not created by users themselves but by tools such as github,
+[gerrit](https://code.google.com/p/gerrit/) and others. These tools often generate merge commit messages that
+violate gitlint's set of rules (a common example is *"Merge:"* being auto-prepended  which can trigger a
+[title-max-length](rules.md#t1-title-max-length) violation)
+and it's not always convenient or desired to change those.
+
+For [squash](https://git-scm.com/docs/git-commit#git-commit---squashltcommitgt) and [fixup](https://git-scm.com/docs/git-commit#git-commit---fixupltcommitgt) commits, the rationale is that these are temporary
+commits that will be squashed into a different commit, and hence the commit messages for these commits are very
+short-lived and not intended to make it into the final commit history. In addition, by prepending *"fixup!"* or
+*"squash!"* to your commit message, certain gitlint rules might be violated
+(e.g. [title-max-length](rules.md#t1-title-max-length)) which is often undesirable.
+
+In case you *do* want to lint these commit messages, you can disable this behavior by setting the
+general ```ignore-merge-commits```, ```ignore-fixup-commits``` or ```ignore-squash-commits``` option to ```false```
 [using one of the various ways to configure gitlint](configuration.md).
 
-
 ## Exit codes ##
 Gitlint uses the exit code as a simple way to indicate the number of violations found.
 Some exit codes are used to indicate special errors as indicated in the table below.

docs/rules.md

@@ -21,7 +21,7 @@ B4    | body-first-line-empty       | >= 0.1          | First line of the body (
 B5    | body-min-length             | >= 0.4          | Body length must be at least 20 characters
 B6    | body-is-missing             | >= 0.4          | Body message must be specified
 B7    | body-changed-file-mention   | >= 0.4          | Body must contain references to certain files if those files are changed in the last commit
-M1    | author-valid-email          | >= 0.8.3        | Author email address must be a valid email address
+M1    | author-valid-email          | >= 0.9          | Author email address must be a valid email address
 
 
 

gitlint/config.py

@@ -67,6 +67,8 @@ def __init__(self):
         self._rules = OrderedDict([(rule_cls.id, rule_cls()) for rule_cls in self.default_rule_classes])
         self._verbosity = options.IntOption('verbosity', 3, "Verbosity")
         self._ignore_merge_commits = options.BoolOption('ignore-merge-commits', True, "Ignore merge commits")
+        self._ignore_fixup_commits = options.BoolOption('ignore-fixup-commits', True, "Ignore fixup commits")
+        self._ignore_squash_commits = options.BoolOption('ignore-squash-commits', True, "Ignore squash commits")
         self._debug = options.BoolOption('debug', False, "Enable debug mode")
         self._extra_path = None
         target_description = "Path of the target git repository (default=current working directory)"
@@ -103,6 +105,24 @@ def ignore_merge_commits(self):
     def ignore_merge_commits(self, value):
         return self._ignore_merge_commits.set(value)
 
+    @property
+    def ignore_fixup_commits(self):
+        return self._ignore_fixup_commits.value
+
+    @ignore_fixup_commits.setter
+    @handle_option_error
+    def ignore_fixup_commits(self, value):
+        return self._ignore_fixup_commits.set(value)
+
+    @property
+    def ignore_squash_commits(self):
+        return self._ignore_squash_commits.value
+
+    @ignore_squash_commits.setter
+    @handle_option_error
+    def ignore_squash_commits(self, value):
+        return self._ignore_squash_commits.set(value)
+
     @property
     def debug(self):
         return self._debug.value
@@ -213,6 +233,8 @@ def __eq__(self, other):
                self.target == other.target and \
                self.extra_path == other.extra_path and \
                self.ignore_merge_commits == other.ignore_merge_commits and \
+               self.ignore_fixup_commits == other.ignore_fixup_commits and \
+               self.ignore_squash_commits == other.ignore_squash_commits and \
                self.debug == other.debug and \
                self.ignore == other.ignore and \
                self._config_path == other._config_path  # noqa
@@ -224,6 +246,8 @@ def __str__(self):
         return_str += u"extra-path: {0}\n".format(self.extra_path)
         return_str += u"ignore: {0}\n".format(",".join(self.ignore))
         return_str += u"ignore-merge-commits: {0}\n".format(self.ignore_merge_commits)
+        return_str += u"ignore-fixup-commits: {0}\n".format(self.ignore_fixup_commits)
+        return_str += u"ignore-squash-commits: {0}\n".format(self.ignore_squash_commits)
         return_str += u"verbosity: {0}\n".format(self.verbosity)
         return_str += u"debug: {0}\n".format(self.debug)
         return_str += u"target: {0}\n".format(self.target)

gitlint/files/gitlint

@@ -1,10 +1,20 @@
 # All these sections are optional, edit this file as you like.
 # [general]
+# Ignore certain rules, you can reference them by their id or by their full name
 # ignore=title-trailing-punctuation, T3
+
 # 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
+
+# By default gitlint will ignore fixup commits. Set to 'false' to disable.
+# ignore-fixup-commits=true
+
+# By default gitlint will ignore squash commits. Set to 'false' to disable.
+# ignore-squash-commits=true
+
 # Enable debug mode (prints more output). Disabled by default.
 # debug=true
 

gitlint/git.py

@@ -106,8 +106,9 @@ class GitCommit(object):
         In the context of gitlint, only the git context and commit message are required.
     """
 
-    def __init__(self, context, message, sha=None, date=None, author_name=None, author_email=None, parents=None,
-                 is_merge_commit=False, changed_files=None):
+    def __init__(self, context, message, sha=None, date=None, author_name=None,  # pylint: disable=too-many-arguments
+                 author_email=None, parents=None, is_merge_commit=None, is_fixup_commit=None,
+                 is_squash_commit=None, changed_files=None):
         self.context = context
         self.message = message
         self.sha = sha
@@ -116,9 +117,13 @@ def __init__(self, context, message, sha=None, date=None, author_name=None, auth
         self.author_email = author_email
         # parent commit hashes
         self.parents = parents or []
-        self.is_merge_commit = is_merge_commit
         self.changed_files = changed_files or []
 
+        # If it's not explicitely specified, we consider a commit a merge commit if its title starts with "Merge"
+        self.is_merge_commit = message.title.startswith(u"Merge") if is_merge_commit is None else is_merge_commit
+        self.is_fixup_commit = message.title.startswith(u"fixup!") if is_fixup_commit is None else is_fixup_commit
+        self.is_squash_commit = message.title.startswith(u"squash!") if is_squash_commit is None else is_squash_commit
+
     def __unicode__(self):
         format_str = u"Author: %s <%s>\nDate:   %s\n%s"  # pragma: no cover
         return format_str % (self.author_name, self.author_email, self.date, ustr(self.message))  # pragma: no cover
@@ -135,7 +140,8 @@ def __eq__(self, other):
                self.sha == other.sha and self.author_name == other.author_name and \
                self.author_email == other.author_email and \
                self.date == other.date and self.parents == other.parents and \
-               self.is_merge_commit == other.is_merge_commit and self.changed_files == other.changed_files  # noqa
+               self.is_merge_commit == other.is_merge_commit and self.is_fixup_commit == other.is_fixup_commit and \
+               self.is_squash_commit == other.is_squash_commit and self.changed_files == other.changed_files  # noqa
 
 
 class GitContext(object):
@@ -154,9 +160,7 @@ def from_commit_msg(commit_msg_str):
         context = GitContext()
         commit_msg_obj = GitCommitMessage.from_full_message(commit_msg_str)
 
-        # For now, we consider a commit a merge commit if its title starts with "Merge"
-        is_merge_commit = commit_msg_obj.title.startswith("Merge")
-        commit = GitCommit(context, commit_msg_obj, is_merge_commit=is_merge_commit)
+        commit = GitCommit(context, commit_msg_obj)
 
         context.commits.append(commit)
         return context
@@ -199,6 +203,7 @@ def from_local_repository(repository_path, refspec=None):
 
             # Create Git commit object with the retrieved info
             commit_msg_obj = GitCommitMessage.from_full_message(commit_msg)
+
             commit = GitCommit(context, commit_msg_obj, sha=sha, author_name=name,
                                author_email=email, date=commit_date, changed_files=changed_files,
                                parents=commit_parents, is_merge_commit=commit_is_merge_commit)

gitlint/lint.py

@@ -64,9 +64,13 @@ def lint(self, commit):
         """ Lint the last commit in a given git context by applying all title, body and general rules. """
         LOG.debug("Linting commit %s", commit.sha or "[SHA UNKNOWN]")
         LOG.debug("Commit Object\n" + ustr(commit))
-        # Skip linting if this is merge commit and if the config is set to ignore those
-        if commit.is_merge_commit and self.config.ignore_merge_commits:
-            return []
+
+        # Skip linting if this is a special commit type that is configured to be ignored
+        ignore_commit_types = ["merge", "squash", "fixup"]
+        for commit_type in ignore_commit_types:
+            if getattr(commit, "is_{0}_commit".format(commit_type)) and \
+               getattr(self.config, "ignore_{0}_commits".format(commit_type)):
+                return []
 
         violations = []
         # determine violations by applying all rules

gitlint/tests/base.py

@@ -29,6 +29,11 @@ def setUp(self):
         logging.getLogger('gitlint').setLevel(logging.DEBUG)
         logging.getLogger('gitlint').handlers = [self.logcapture]
 
+        # Make sure we don't propagate anything to child loggers, we need to do this explicitely here
+        # because if you run a specific test file like test_lint.py, we won't be calling the setupLogging() method
+        # in gitlint.cli that normally takes care of this
+        logging.getLogger('gitlint').propagate = False
+
     @staticmethod
     def get_sample_path(filename=""):
         # Don't join up empty files names because this will add a trailing slash
@@ -71,10 +76,13 @@ def gitcontext(commit_msg_str, changed_files=None):
         return gitcontext
 
     @staticmethod
-    def gitcommit(commit_msg_str, changed_files=None):
+    def gitcommit(commit_msg_str, changed_files=None, **kwargs):
         """ Utility method to easily create git commit given a commit msg string and an optional set of changed files"""
         gitcontext = BaseTestCase.gitcontext(commit_msg_str, changed_files)
-        return gitcontext.commits[-1]
+        commit = gitcontext.commits[-1]
+        for attr, value in kwargs.items():
+            setattr(commit, attr, value)
+        return commit
 
     def assert_logged(self, lines):
         """ Asserts that a certain list of messages has been logged """

gitlint/tests/expected/debug_configuration_output1

@@ -4,6 +4,8 @@ config-path: {config_path}
 extra-path: None
 ignore: title-trailing-whitespace,B2
 ignore-merge-commits: False
+ignore-fixup-commits: True
+ignore-squash-commits: True
 verbosity: 1
 debug: True
 target: {target}

gitlint/tests/samples/commit_message/fixup

@@ -0,0 +1 @@
+fixup! WIP: This is a fixup cömmit with violations.

gitlint/tests/samples/commit_message/squash

@@ -0,0 +1,3 @@
+squash! WIP: This is a squash cömmit with violations.
+
+Body töo short