Skip to content

Commit

Permalink
Assign codes to messages and hook up --explain
Browse files Browse the repository at this point in the history
  • Loading branch information
erik committed Jan 1, 2019
1 parent dcee378 commit 411a055
Show file tree
Hide file tree
Showing 11 changed files with 129 additions and 26 deletions.
3 changes: 2 additions & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Catch unsafe SQL migrations.

.. code:: sql
-- sql/migration.sql:4:46 ERROR: column "uh_oh" has a disallowed constraint
-- sql/migration.sql:4:46 ERROR: column "uh_oh" has a disallowed constraint [1001]
ALTER TABLE big_table ADD COLUMN uh_oh integer DEFAULT 0;
^
Expand Down Expand Up @@ -138,6 +138,7 @@ bindings <https://github.com/lelit/pglast/tree/master/pglast/enums>`__.
# Define the different message types that this rule can return
class TableNotLoudEnough(Message):
"""Add more details about the message here"""
CODE = 9876
TEMPLATE = 'table "{name}" not LOUD ENOUGH'
def enable(self, root_ctx, config):
Expand Down
26 changes: 26 additions & 0 deletions squabble/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
Options:
-c --config=PATH Path to configuration file
-e --explain=CODE Show explanation of a rule's message code.
-h --help Show this screen
-p --preset=PRESET Start with a base preset rule configuration
-P --list-presets List the available preset configurations
Expand All @@ -29,6 +30,7 @@
from colorama import Style

import squabble
import squabble.message
from squabble import config, lint, logger, rule, reporter


Expand Down Expand Up @@ -68,6 +70,9 @@ def dispatch_args(args):
if args['--show-rule']:
return show_rule(name=args['--show-rule'])

if args['--explain']:
return explain_message(code=args['--explain'])

return run_linter(base_config, args['PATHS'])


Expand Down Expand Up @@ -148,6 +153,27 @@ def list_rules():
}))


def explain_message(code):
"""Print out the more detailed explanation of the given message code."""
try:
code = int(code)
cls = squabble.message.Registry.by_code(code)
except (ValueError, KeyError):
sys.exit('{bold}Unknown message code:{reset} {code}'.format(
bold=Style.BRIGHT,
reset=Style.RESET_ALL,
code=code
))

print('{bold}{name}{reset}'.format(
bold=Style.BRIGHT,
reset=Style.RESET_ALL,
name=cls.__name__
))

print('\t', cls.explain() or 'No additional info.', sep='')


def list_presets():
"""Print out all the preset configurations."""
for name, preset in config.PRESETS.items():
Expand Down
77 changes: 74 additions & 3 deletions squabble/message.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,63 @@
import re

from squabble import logger, SquabbleException


class DuplicateMessageCodeException(SquabbleException):
def __init__(self, dupe):
original = Registry.by_code(dupe.CODE)
message = 'Message %s has the same code as %s' % (dupe, original)

super().__init__(message)


class Registry:
"""
Singleton which maps message code values to classes.
>>> class MyMessage(Message):
... '''My example message.'''
... TEMPLATE = '...'
... CODE = 5678
>>> cls = Registry.by_code(5678)
>>> cls.explain()
'My example message.'
>>> cls is MyMessage
True
Duplicate codes are not allowed, and will throw an exception.
>>> class MyDuplicateMessage(Message):
... CODE = 5678
Traceback (most recent call last):
...
squabble.message.DuplicateMessageCodeException: ...
"""

_MAP = {}
_CODE_COUNTER = 9000

@classmethod
def register(cls, msg):
if not hasattr(msg, 'CODE'):
setattr(msg, 'CODE', cls._next_code())
logger.info('assigning code %s to %s', msg.CODE, msg)

# Don't allow duplicates
if msg.CODE in cls._MAP:
raise DuplicateMessageCodeException(msg)

cls._MAP[msg.CODE] = msg

@classmethod
def by_code(cls, code):
return cls._MAP[code]

@classmethod
def _next_code(cls):
cls._CODE_COUNTER += 1
return cls._CODE_COUNTER


class Message:
"""
Expand All @@ -10,11 +68,17 @@ class Message:
as a class member variable named ``TEMPLATE``, which is used to
display a brief explanation on the command line.
Messages may also have a ``CODE`` class member, which is used to
identify the message. The actual value doesn't matter much, as
long as it is unique among all the loaded ``Message``s. If no
``CODE`` is defined, one will be assigned.
>>> class TooManyColumns(Message):
... '''
... This may indicate poor design, consider multiple tables instead.
... '''
... TEMPLATE = 'table "{table}" has > {limit} columns'
... CODE = 1234
>>> message = TooManyColumns(table='foo', limit=30)
>>> message.format()
'table "foo" has > 30 columns'
Expand All @@ -26,10 +90,16 @@ class Message:
def __init__(self, **kwargs):
self.kwargs = kwargs

def __init_subclass__(cls, **kwargs):
"""Assign unique (locally, not globally) message codes."""
super().__init_subclass__(**kwargs)
Registry.register(cls)

def format(self):
return self.TEMPLATE.format(**self.kwargs)

def explain(self):
@classmethod
def explain(cls):
"""
Provide more context around this message.
Expand All @@ -44,10 +114,10 @@ def explain(self):
>>> NoDocString().explain() is None
True
"""
if not self.__doc__:
if not cls.__doc__:
return None

return self.__doc__.strip()
return cls.__doc__.strip()

def asdict(self):
name = self.__class__.__name__
Expand All @@ -58,4 +128,5 @@ def asdict(self):
'message_text': self.format(),
'message_template': self.TEMPLATE,
'message_params': self.kwargs,
'message_code': self.CODE
}
10 changes: 5 additions & 5 deletions squabble/reporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,8 @@ def _issue_info(issue, file_contents):

# Partially pre-format the message since the color codes will be static.
_COLOR_FORMAT = '{bold}{{file}}:{reset}{{line}}:{{column}}{reset} '\
'{red}{{severity}}{reset}: {{message_formatted}}'.format(**{
'bold': Style.BRIGHT,
'red': Fore.RED,
'reset': Style.RESET_ALL,
})
'{red}{{severity}}{reset}: {{message_formatted}}'\
.format(bold=Style.BRIGHT, red=Fore.RED, reset=Style.RESET_ALL)


@reporter("plain")
Expand All @@ -155,6 +152,9 @@ def color_reporter(issue, file_contents):

output = [_COLOR_FORMAT.format(**info)]

if 'message_code' in info:
output[0] += ' [{message_code}]'.format(**info)

if info['line_text'] != '':
arrow = ' ' * info['column'] + '^'
output.append(info['line_text'])
Expand Down
3 changes: 2 additions & 1 deletion squabble/rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ def _load_builtin_rules():
"""Load the rules that ship with squabble (squabble/rules/*.py)"""
modules = glob.glob(os.path.dirname(__file__) + '/rules/*.py')

for mod in modules:
# Sort the modules to guarantee stable ordering
for mod in sorted(modules):
mod_name = os.path.basename(mod)[:-3]

if not os.path.isfile(mod) or mod_name.startswith('__'):
Expand Down
23 changes: 12 additions & 11 deletions squabble/rules/add_column_disallow_constraint.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@

class AddColumnDisallowConstraints(BaseRule):
"""
Prevent adding a column with certain constraints to an existing table
Prevent adding a column with certain constraints to an existing table.
Configuration:
.. code-block:: json
::
{
"AddColumnDisallowConstraints": {
Expand Down Expand Up @@ -54,7 +54,7 @@ class ConstraintNotAllowed(Message):
possibly dangerous overhead to confirm the referential integrity of
each row.
"""

CODE = 1004
TEMPLATE = 'column "{col}" has a disallowed constraint'

def enable(self, ctx, config):
Expand All @@ -80,16 +80,17 @@ def _check(self, ctx, node, disallowed_constraints):
"""
Node is an `AlterTableCmd`:
{
'AlterTableCmd': {
'def': {
'ColumnDef': {
'colname': 'bar',
'constraints': [{'Constraint': {'contype': 2, 'location': 35}}]
::
{
'AlterTableCmd': {
'def': {
'ColumnDef': {
'colname': 'bar',
'constraints': [{'Constraint': {'contype': 2}}]
}
}
}
}
}
}
"""

# We only care about adding a column
Expand Down
2 changes: 1 addition & 1 deletion squabble/rules/disallow_change_column_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class ChangeTypeNotAllowed(Message):
-- Deploy server code to point to new column
ALTER TABLE foo DROP COLUMN bar_old;
"""

CODE = 1003
TEMPLATE = 'cannot change type of existing column "{col}"'

def enable(self, ctx, _config):
Expand Down
1 change: 1 addition & 0 deletions squabble/rules/disallow_rename_enum_value.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class RenameNotAllowed(Message):
Renaming an existing enum value may be backwards compatible
with code that is live in production.
"""
CODE = 1000
TEMPLATE = 'cannot rename existing enum value "{value}"'

def enable(self, ctx, _config):
Expand Down
6 changes: 4 additions & 2 deletions squabble/rules/require_columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,24 +102,26 @@ class RequireColumns(BaseRule):
Configuration:
.. code-block:: json
::
{
"RequireColumns": {
"required": ["column_foo,column_type", "column_bar"]
}
}
If a column type is specified (like `column_foo` in the example
If a column type is specified (like ``column_foo`` in the example
configuration), the linter will make sure that the types match.
Otherwise, only the presence of the column will be checked.
"""

class MissingRequiredColumn(Message):
CODE = 1005
TEMPLATE = '"{tbl}" missing required column "{col}"'

class ColumnWrongType(Message):
CODE = 1006
TEMPLATE = '"{tbl}.{col}" has type "{actual}" expected "{required}"'

def enable(self, ctx, config):
Expand Down
2 changes: 1 addition & 1 deletion squabble/rules/require_concurrent_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class IndexNotConcurrent(Message):
CREATE INDEX CONCURRENTLY users_by_name ON users(name);
"""

CODE = 1001
TEMPLATE = 'index "{name}" not created `CONCURRENTLY`'

def enable(self, ctx, config):
Expand Down
2 changes: 1 addition & 1 deletion squabble/rules/require_primary_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class MissingPrimaryKey(Message):
If no single column will uniquely identify a row, creating a composite
primary key is also possible.
"""

CODE = 1002
TEMPLATE = 'table "{tbl}" does not name a primary key'

def enable(self, ctx, _config):
Expand Down

0 comments on commit 411a055

Please sign in to comment.