Line-based ignores

squabble/cli.py

@@ -105,8 +105,9 @@ def run_linter(base_config, paths, expanded):
 
     issues = []
     for file_name, contents in files:
-        file_config = config.apply_file_config(base_config, contents)
-        if file_config is None:
+        file_config = config.apply_file_config(base_config, file_name, contents)
+        # The file should be skipped if no specific line is ignored
+        if file_config.skip.get(file_name) == []:
             continue
         issues += lint.check_file(file_config, file_name, contents)
 

squabble/config.py

@@ -15,15 +15,17 @@
 Config = collections.namedtuple('Config', [
     'reporter',
     'plugins',
-    'rules'
+    'rules',
+    'skip'
 ])
 
 
 # TODO: Move these out somewhere else, feels gross to have them hardcoded.
 DEFAULT_CONFIG = dict(
     reporter='plain',
     plugins=[],
-    rules={}
+    rules={},
+    skip={}
 )
 
 PRESETS = {
@@ -87,6 +89,11 @@
     }
 }
 
+COMMENT_RE = re.compile(
+    r'--\s*'
+    r'(?:squabble-)?(enable|disable)?(-next-line)?'
+    r'(?::\s*(\w+)(.*?))?'
+    r'$', re.I)
 
 class UnknownPresetException(SquabbleException):
     """Raised when user tries to apply a preset that isn't defined."""
@@ -189,33 +196,40 @@ def load_config(config_file, preset_names=None, reporter_name=None):
     return Config(
         reporter=reporter_name or config.get('reporter', base.reporter),
         plugins=config.get('plugins', base.plugins),
-        rules=rules
+        rules=rules,
+        skip=config.get('skip', {})
     )
 
 
-def apply_file_config(base, contents):
+def apply_file_config(base, file_name, contents):
     """
     Given a base configuration object and the contents of a file,
     return a new config that applies any file-specific rule
     additions/deletions.
-
-    Returns ``None`` if the file should be skipped.
     """
     # Operate on a copy so we don't mutate the base config
     file_rules = copy.deepcopy(base.rules)
+    skips = copy.deepcopy(base.skip)
 
     rules = _extract_file_rules(contents)
 
-    if rules['skip_file']:
-        return None
-
     for rule, opts in rules['enable'].items():
         file_rules[rule] = opts
 
     for rule in rules['disable']:
         del file_rules[rule]
 
-    return base._replace(rules=file_rules)
+    for line in rules['skip_lines']:
+        if skips.get(file_name) is None:
+            skips[file_name] = []
+        skips[file_name].append(line)
+
+    # Handle 'skip_file' after 'skip_lines', as it should be seen as
+    # an override
+    if rules['skip_file']:
+        skips = {**skips, file_name: []}
+
+    return base._replace(rules=file_rules, skip=skips)
 
 
 def _extract_file_rules(text):
@@ -238,26 +252,27 @@ def _extract_file_rules(text):
     rules = {
         'enable': {},
         'disable': [],
-        'skip_file': False
+        'skip_file': False,
+        'skip_lines': []
     }
 
-    comment_re = re.compile(
-        r'--\s*'
-        r'(?:squabble-)?(enable|disable)'
-        r'(?::\s*(\w+)(.*?))?'
-        r'$', re.I)
-
-    for line in text.splitlines():
+    for line_num, line in enumerate(text.splitlines()):
         line = line.strip()
-
-        m = re.match(comment_re, line)
+        m = re.match(COMMENT_RE, line)
         if m is None:
             continue
 
-        action, rule, opts = m.groups()
+        action, ctx, rule, opts = m.groups()
+        line_specific = ctx is not None
 
         if action == 'disable' and not rule:
-            rules['skip_file'] = True
+            if not line_specific:
+                rules['skip_file'] = True
+            else:
+                # Disregard specific rule for now
+                # Starts with 0, +1 additional line because
+                # we want to skip the *next* line
+                rules['skip_lines'].append(line_num + 2)
 
         elif action == 'disable':
             rules['disable'].append(rule)

squabble/lint.py

@@ -13,7 +13,10 @@
     'node',
     'file',
     'severity',
-    'location'
+    'line_text',
+    'line',
+    'column',
+    'abs_location'
 ])
 
 # Make all the fields nullable
@@ -73,7 +76,7 @@ def check_file(config, name, contents):
     ``name``.
     """
     rules = _configure_rules(config.rules)
-    s = Session(rules, contents, file_name=name)
+    s = Session(rules, config.skip, contents, name)
     return s.lint()
 
 
@@ -83,15 +86,27 @@ class Session:
     class exists mainly to hold the list of issues returned by the enabled
     rules.
     """
-    def __init__(self, rules, sql_text, file_name):
+
+    def __init__(self, rules, skips, sql_text, file_name):
         self._rules = rules
+        self._skips = skips
         self._sql = sql_text
         self._issues = []
         self._file_name = file_name
 
     def report_issue(self, issue):
-        i = issue._replace(file=self._file_name)
-        self._issues.append(i)
+        line_text, line, column = _resolve_location(
+            _location_for_issue(issue), 
+            self._sql
+        )
+        i = issue._replace(file=self._file_name)._replace(
+            line_text=line_text, line=line, column=column
+        )
+
+        should_skip = line in self._skips.get(self._file_name, [])
+
+        if not should_skip:
+            self._issues.append(i)
 
     def lint(self):
         """
@@ -111,7 +126,7 @@ def lint(self):
             root_ctx.report_issue(LintIssue(
                 severity=Severity.CRITICAL,
                 message_text=exc.args[0],
-                location=exc.location
+                abs_location=exc.location
             ))
 
         return self._issues
@@ -203,3 +218,56 @@ def report(self, message, node=None, severity=None):
             # This is filled in later
             file=None,
         ))
+
+def _location_for_issue(issue):
+    """
+    Return the offset into the file for this issue, or None if it
+    cannot be determined.
+    """
+    if issue.node and issue.node.location != pglast.Missing:
+        return issue.node.location.value
+
+    if issue.abs_location is not None:
+        return issue.abs_location
+
+    return None
+
+
+def _resolve_location(location, contents):
+    """
+    Given a location and the contents of the file, 
+    return the ``(line_str, line, column)`` that node is located at,
+    or ``('', 1, 0)``.
+
+    :param issue:
+    :type issue: :int:
+    :param contents: Full contents of the file being linted, as a string.
+    :type contents: str
+
+    >>> from squabble.lint import LintIssue
+
+    >>> sql = '1234\\n678\\nABCD'
+    >>> _resolve_location(8, sql)
+    ('678', 2, 3)
+
+    >>> sql = '1\\r\\n\\r\\n678\\r\\nBCD'
+    >>> _resolve_location(7, sql)
+    ('678', 3, 2)
+    """
+    if location is None or location >= len(contents):
+        return ("", 1, 0)
+
+    # line number is number of newlines in the file before this
+    # locationation, 1 indexed.
+    line_num = contents[:location].count("\n") + 1
+
+    # Search forwards/backwards for the first newline before and first
+    # newline after this point.
+    line_start = contents.rfind("\n", 0, location) + 1
+    line_end = contents.find("\n", location)
+
+    # Strip out \r so we can treat \r\n and \n the same way
+    line = contents[line_start:line_end].replace("\r", "")
+    column = location - line_start
+
+    return (line, line_num, column)

squabble/reporter.py

@@ -82,62 +82,6 @@ def report(reporter_name, issues, files):
             _print_err(line)
 
 
-def _location_for_issue(issue):
-    """
-    Return the offset into the file for this issue, or None if it
-    cannot be determined.
-    """
-    if issue.node and issue.node.location != pglast.Missing:
-        return issue.node.location.value
-
-    return issue.location
-
-
-def _issue_to_file_location(issue, contents):
-    """
-    Given an issue (which may or may not have a :class:`pglast.Node` with a
-    ``location`` field) and the contents of the file containing that
-    node, return the ``(line_str, line, column)`` that node is located at,
-    or ``('', 1, 0)``.
-
-    :param issue:
-    :type issue: :class:`squabble.lint.LintIssue`
-    :param contents: Full contents of the file being linted, as a string.
-    :type contents: str
-
-    >>> from squabble.lint import LintIssue
-
-    >>> issue = LintIssue(location=8, file='foo')
-    >>> sql = '1234\\n678\\nABCD'
-    >>> _issue_to_file_location(issue, sql)
-    ('678', 2, 3)
-
-    >>> issue = LintIssue(location=7, file='foo')
-    >>> sql = '1\\r\\n\\r\\n678\\r\\nBCD'
-    >>> _issue_to_file_location(issue, sql)
-    ('678', 3, 2)
-    """
-    loc = _location_for_issue(issue)
-
-    if loc is None or loc >= len(contents):
-        return ('', 1, 0)
-
-    # line number is number of newlines in the file before this
-    # location, 1 indexed.
-    line_num = contents[:loc].count('\n') + 1
-
-    # Search forwards/backwards for the first newline before and first
-    # newline after this point.
-    line_start = contents.rfind('\n', 0, loc) + 1
-    line_end = contents.find('\n', loc)
-
-    # Strip out \r so we can treat \r\n and \n the same way
-    line = contents[line_start:line_end].replace('\r', '')
-    column = loc - line_start
-
-    return(line, line_num, column)
-
-
 def _print_err(msg):
     print(msg, file=sys.stderr)
 
@@ -149,17 +93,13 @@ def _format_message(issue):
     return issue.message.format()
 
 
-def _issue_info(issue, file_contents):
+def _issue_info(issue):
     """Return a dictionary of metadata for an issue."""
-    line, line_num, column = _issue_to_file_location(issue, file_contents)
     formatted = _format_message(issue)
 
     return {
         **issue._asdict(),
         **(issue.message.asdict() if issue.message else {}),
-        'line_text': line,
-        'line': line_num,
-        'column': column,
         'message_formatted': formatted,
         'severity': issue.severity.name,
     }
@@ -176,7 +116,7 @@ def _issue_info(issue, file_contents):
 @reporter("plain")
 def plain_text_reporter(issue, file_contents):
     """Simple single-line output format that is easily parsed by editors."""
-    info = _issue_info(issue, file_contents)
+    info = _issue_info(issue)
     return [
         _SIMPLE_FORMAT.format(**info)
     ]
@@ -196,7 +136,7 @@ def color_reporter(issue, file_contents):
     Extension of :func:`squabble.reporter.plain_text_reporter`, uses
     ANSI color and shows error location.
     """
-    info = _issue_info(issue, file_contents)
+    info = _issue_info(issue)
     info['severity'] = '{color}{severity}{reset}'.format(
         color=_SEVERITY_COLOR[issue.severity],
         severity=issue.severity.name,
@@ -254,7 +194,7 @@ def sqlint_reporter(issue, file_contents):
 
     error_level = {Severity.HIGH, Severity.CRITICAL}
 
-    info = _issue_info(issue, file_contents)
+    info = _issue_info(issue)
     info['severity'] = 'ERROR' if issue.severity in error_level else 'WARNING'
 
     return [