Prefix internal functions with _

README.rst

@@ -5,17 +5,17 @@ Catch unsafe SQL migrations.
 
 .. code:: console
 
-   $ squabble sql/migration.sql
-   sql/migration.sql:4:46 ERROR: column "uh_oh" has a disallowed constraint [1004]
-   ALTER TABLE big_table ADD COLUMN uh_oh integer DEFAULT 0;
-                                                  ^
-   # Use --explain to get more information on a lint violation
-   $ squabble --explain 1004
-   ConstraintNotAllowed
-        When adding a column to an existing table, certain constraints can have
-        unintentional side effects, like locking the table or introducing
-        performance issues.
-        ...
+  $ squabble sql/migration.sql
+  sql/migration.sql:4:46 ERROR: column "uh_oh" has a disallowed constraint [1004]
+  ALTER TABLE big_table ADD COLUMN uh_oh integer DEFAULT 0;
+                                                 ^
+  # Use --explain to get more information on a lint violation
+  $ squabble --explain 1004
+  ConstraintNotAllowed
+       When adding a column to an existing table, certain constraints can have
+       unintentional side effects, like locking the table or introducing
+       performance issues.
+       ...
 
 Squabble works to help automate the process of reviewing SQL migrations
 by catching simple mistakes, such as:
@@ -42,9 +42,9 @@ Installation
    $ pip3 install squabble
    $ squabble --help
 
-..
+.. note::
 
-   Note: Squabble is only supported on Python 3.5+
+   Squabble is only supported on Python 3.5+
 
 If you’d like to install from source:
 

docs/conf.py

@@ -14,6 +14,7 @@
 #
 import os
 import sys
+
 sys.path.insert(0, os.path.abspath('..'))
 
 

docs/plugins.rst

@@ -89,10 +89,10 @@ line flag ::
 
       ...
 
-Lint context
-~~~~~~~~~~~~
+Context
+~~~~~~~
 
-Each instance of :class:`squabble.lint.LintContext` holds the callback
+Each instance of :class:`squabble.lint.Context` holds the callback
 functions that have been registered at or below a particular node in the
 abstract syntax tree, as well as being responsible for reporting any messages
 that get raised.

setup.py

@@ -1,6 +1,5 @@
 from setuptools import setup
 
-
 __version__ = '0.1.0'
 
 

squabble/cli.py

@@ -24,14 +24,14 @@
 import json
 import os.path
 import sys
-from pkg_resources import get_distribution
 
 import docopt
 from colorama import Style
+from pkg_resources import get_distribution
 
 import squabble
 import squabble.message
-from squabble import config, lint, logger, rule, reporter
+from squabble import config, lint, logger, reporter, rule
 
 
 def main():
@@ -92,7 +92,7 @@ def run_linter(base_config, paths):
     reporter.report(base_config.reporter, issues)
 
     # Make sure we have an error status if something went wrong.
-    return 1 if len(issues) > 0 else 0
+    return 1 if issues else 0
 
 
 def collect_files(paths):

squabble/config.py

@@ -5,8 +5,7 @@
 import re
 import subprocess
 
-from squabble import logger, SquabbleException
-
+from squabble import SquabbleException, logger
 
 Config = collections.namedtuple('Config', [
     'reporter',
@@ -41,16 +40,27 @@
 
 
 class UnknownPresetException(SquabbleException):
+    """Raised when user tries to apply a preset that isn't defined."""
     def __init__(self, preset):
         super().__init__('unknown preset: "%s"' % preset)
 
 
 def discover_config_location():
+    """
+    Try to locate a config file in some likely locations.
+
+    Used when no config path is specified explicitly. In order, this
+    will check for a file named ``.squabblerc``:
+
+    - in the current directory.
+    - in the root of the repository (if working in a git repo).
+    - in the user's home directory.
+    """
     logger.debug('No config file given, trying to discover')
 
     possible_dirs = [
         '.',
-        get_vcs_root(),
+        _get_vcs_root(),
         os.path.expanduser('~')
     ]
 
@@ -67,7 +77,7 @@ def discover_config_location():
     return None
 
 
-def get_vcs_root():
+def _get_vcs_root():
     """
     Return the path to the root of the Git repository for the current directory,
     or empty string if not in a repository.
@@ -95,7 +105,7 @@ def get_base_config(preset_name=None):
     })
 
 
-def parse_config_file(config_file):
+def _parse_config_file(config_file):
     if not config_file:
         return {}
 
@@ -104,8 +114,15 @@ def parse_config_file(config_file):
 
 
 def load_config(config_file, preset_name=None):
+    """
+    Load configuration from a file, optionally applying a predefined
+    set of rules.
+
+    :param config_file: Path to JSON file containing user configuration.
+    :type config_file: str
+    """
     base = get_base_config(preset_name)
-    config = parse_config_file(config_file)
+    config = _parse_config_file(config_file)
 
     rules = copy.deepcopy(base.rules)
     for name, rule in config.get('rules', {}).items():
@@ -126,7 +143,7 @@ def apply_file_config(base, file_name):
     # Operate on a copy so we don't mutate the base config
     file_rules = copy.deepcopy(base.rules)
 
-    rules = parse_file_rules(file_name)
+    rules = _parse_file_rules(file_name)
 
     for rule, opts in rules['enable'].items():
         file_rules[rule] = opts
@@ -137,20 +154,20 @@ def apply_file_config(base, file_name):
     return base._replace(rules=file_rules)
 
 
-def parse_file_rules(file_name):
+def _parse_file_rules(file_name):
     with open(file_name, 'r') as fp:
         text = fp.read()
 
-    return extract_file_rules(text)
+    return _extract_file_rules(text)
 
 
-def extract_file_rules(text):
+def _extract_file_rules(text):
     """
     Try to extract any file-level rule additions/suppressions.
 
     Valid lines are SQL line comments that enable or disable specific rules.
 
-    >>> rules = extract_file_rules('-- enable:rule1 arr=a,b,c')
+    >>> rules = _extract_file_rules('-- enable:rule1 arr=a,b,c')
     >>> rules['disable']
     []
     >>> rules['enable']

squabble/lint.py

@@ -6,7 +6,6 @@
 
 from squabble.rules import Registry
 
-
 _LintIssue = collections.namedtuple('LintIssue', [
     'message',
     'message_text',
@@ -24,11 +23,9 @@ class LintIssue(_LintIssue):
     pass
 
 
-def parse_file(file_name):
-    with open(file_name, 'r') as fp:
-        contents = fp.read()
-
-    return pglast.Node(pglast.parse_sql(contents))
+def _parse_string(text):
+    """Use ``pglast`` to turn ``text`` into a SQL AST."""
+    return pglast.Node(pglast.parse_sql(text))
 
 
 def _configure_rules(rule_config):
@@ -43,11 +40,15 @@ def _configure_rules(rule_config):
 
 def check_file(config, file_name):
     """
-    Return a list of lint issues from using the given config to lint
-    `file_name`.
+    Return a list of lint issues from using ``config`` to lint
+    ``file_name``.
     """
+
+    with open(file_name, 'r') as fp:
+        text = fp.read()
+
     rules = _configure_rules(config.rules)
-    s = Session(rules, file_name)
+    s = Session(rules, text, file_name)
     return s.lint()
 
 
@@ -57,23 +58,24 @@ class Session:
     class exists mainly to hold the list of issues returned by the enabled
     rules.
     """
-    def __init__(self, rules, file_name):
+    def __init__(self, rules, sql_text, file_name):
         self._rules = rules
-        self._file = file_name
+        self._sql = sql_text
         self._issues = []
+        self._file_name = file_name
 
     def report_issue(self, issue):
-        i = issue._replace(file=self._file)
+        i = issue._replace(file=self._file_name)
         self._issues.append(i)
 
     def lint(self):
-        root_ctx = LintContext(self)
+        root_ctx = Context(self)
 
         for rule, config in self._rules:
             rule.enable(root_ctx, config)
 
         try:
-            ast = parse_file(self._file)
+            ast = _parse_string(self._sql)
             root_ctx.traverse(ast)
 
         except pglast.parser.ParseError as exc:
@@ -86,7 +88,7 @@ def lint(self):
         return self._issues
 
 
-class LintContext:
+class Context:
     """
     Contains the node tag callback hooks enabled at or below the `parent_node`
     passed to the call to `traverse`.
@@ -95,7 +97,7 @@ class LintContext:
     >>> ast = pglast.Node(pglast.parse_sql('''
     ...   CREATE TABLE foo (id INTEGER PRIMARY KEY);
     ... '''))
-    >>> ctx = LintContext(session=...)
+    >>> ctx = Context(session=...)
     >>>
     >>> def create_stmt(child_ctx, node):
     ...     print('create stmt')
@@ -130,7 +132,7 @@ def traverse(self, parent_node):
             if tag not in self._hooks:
                 continue
 
-            child_ctx = LintContext(self._session)
+            child_ctx = Context(self._session)
             for hook in self._hooks[tag]:
                 hook(child_ctx, node)
 
@@ -152,7 +154,7 @@ def register(self, node_tag, fn):
         Register `fn` to be called whenever `node_tag` node is visited.
 
         >>> session = ...
-        >>> ctx = LintContext(session)
+        >>> ctx = Context(session)
         >>> ctx.register('CreateStmt', lambda ctx, node: ...)
         """
         if node_tag not in self._hooks:

squabble/message.py

@@ -1,4 +1,4 @@
-from squabble import logger, SquabbleException
+from squabble import SquabbleException, logger
 
 
 class DuplicateMessageCodeException(SquabbleException):