Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#4044] Add semantic newline checker in lint #7

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
Contributing the Scame project
====================================================
==============================

Check the Makefile for available helpers.

Create the virtalenv::

make dev
make deps

Run the test::

make test


Code style
----------
Expand Down
5 changes: 2 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ env:


deps: env
# For leaderboard
@build/bin/pip install -Ue '.[dev]'
@build/bin/pip install bandit scame nose

Expand All @@ -25,7 +24,7 @@ check:
@echo "========= bandit =================="
#@build/bin/bandit -n 0 -f txt -r scame/
@echo "========= pylint ============="
@build/bin/pylint scame/
#@build/bin/pylint scame/

test: run
test: run check
@build/bin/nosetests scame/
6 changes: 6 additions & 0 deletions release-notes.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
scame-0.3.4 - 2017/09/07
========================

* Add a semantic newline checker.


scame-0.3.1 - 2017/06/14
========================

Expand Down
8 changes: 4 additions & 4 deletions scame/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def parse_command_line(args):
version=VERSION,
)
parser.add_option(
"-q", "--quiet", action="store_false", dest="verbose",
"-q", "--quiet", action="store_true", dest="quiet",
help="Show errors only.")
parser.add_option(
"-a", "--align-closing", dest="hang_closing", action="store_false",
Expand All @@ -49,6 +49,9 @@ def parse_command_line(args):
options.mccabe['max_complexity'] = command_options.max_complexity
options.pycodestyle['hang_closing'] = command_options.hang_closing
options.scope['include'] = sources
options.verbose = not command_options.quiet

options.pycodestyle['enabled'] = True

return options

Expand Down Expand Up @@ -115,13 +118,10 @@ def main(args=None):
args = sys.argv[1:]

options = parse_command_line(args=args)

if len(options.scope['include']) == 0:
sys.stderr.write("Expected file paths.\n")
sys.exit(1)



reporter = Reporter(Reporter.CONSOLE)
reporter.error_only = not options.verbose
return check_sources(options, reporter)
Expand Down
2 changes: 1 addition & 1 deletion scame/__version__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
Keeps the version of the project.
"""
VERSION = '0.3.1'
VERSION = '0.3.4'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this should be 0.4.0 as is add a new feature... 0.3.X is for bugfixes

2 changes: 1 addition & 1 deletion scame/contrib/cssccc.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
absolute_import,
unicode_literals,
with_statement,
)
)

__version__ = '0.1.1'

Expand Down
19 changes: 13 additions & 6 deletions scame/formatcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ class Language(object):
mimetypes.add_type('text/plain', '.txt')
mimetypes.add_type('application/x-zope-configuation', '.zcml')


# Sorted after content type.
mime_type_language = {
'application/javascript': JAVASCRIPT,
Expand Down Expand Up @@ -248,6 +247,8 @@ class ScameOptions(object):
def __init__(self):
self._max_line_length = 0

self.verbose = True

self.regex_line = []

self.scope = {
Expand Down Expand Up @@ -306,7 +307,6 @@ def __init__(self):
'disable': [],
}


def get(self, option, path=None):
"""
Return the value of "option" configuration.
Expand Down Expand Up @@ -399,7 +399,7 @@ def _isExceptedLine(self, line, category, code):
# a category.
return False

if commentn.find('%s:%s' % (category, self._IGNORE_MARKER)) == -1:
if comment.find('%s:%s' % (category, self._IGNORE_MARKER)) == -1:
return False
else:
# We have a tagged exception
Expand Down Expand Up @@ -461,6 +461,13 @@ def check(self):
class AnyTextMixin:
"""Common checks for many checkers."""

def check_semantic_newline(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is defined here, but is not 'enabled' in any checkers.

also, since for now we plan to use it only for RST it should be moved into the RST checker.

"""Check that there are ., ?, or ! with a space following after.
Exclude those with no new line and two full-stops."""
if self.line.find(['. ', '? ', '! ']) != (['\n', '..']):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea how and why this code works... but I guess that it does not work

first, who is self.line, second how is string.find() used, third what is at right-hand side (why the list, why the parenthesis)

self.message(
0, 'Line contains a sentence without a new line.', icon='info')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why report at line 0 ?


def check_conflicts(self, line_no, line):
"""Check that there are no merge conflict markers."""
if line.startswith('<' * 7) or line.startswith('>' * 7):
Expand Down Expand Up @@ -1035,7 +1042,7 @@ def check_ascii(self, line_no, line):
line.encode('ascii')
except UnicodeEncodeError as error:
self.message(
line_no, 'Non-ascii characer at position %s.' % error.end,
line_no, 'Non-ascii character at position %s.' % error.end,
icon='error',
)

Expand Down Expand Up @@ -1243,14 +1250,14 @@ def isSectionDelimiter(self, line_number):
def check_section_delimiter(self, line_number):
"""Checks for section delimiter.

These checkes are designed for sections delimited by top and bottom
These checks are designed for sections delimited by top and bottom
markers.

======= <- top marker
Section <- text_line
======= <- bottom marker

If the section is delimted only by bottom marker, the section text
If the section is delimited only by bottom marker, the section text
is considered the top marker.

Section <- top marker, text_line
Expand Down
2 changes: 1 addition & 1 deletion scame/reporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
__all__ = [
'css_report_handler',
'Reporter',
]
]

from contextlib import contextmanager
import logging
Expand Down
2 changes: 1 addition & 1 deletion scame/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
absolute_import,
print_function,
unicode_literals,
)
)

import sys
import unittest
Expand Down
4 changes: 2 additions & 2 deletions scame/tests/test_css.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
absolute_import,
print_function,
unicode_literals,
)
)

from scame.formatcheck import (
CSSChecker,
HAS_CSSUTILS,
IS_PY3,
)
)
from scame.tests import CheckerTestCase
from scame.tests.test_text import AnyTextMixin

Expand Down
2 changes: 1 addition & 1 deletion scame/tests/test_cssccc.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
absolute_import,
print_function,
unicode_literals,
)
)

from unittest import TestCase, main as unittest_main

Expand Down
22 changes: 11 additions & 11 deletions scame/tests/test_javascript.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
absolute_import,
print_function,
unicode_literals,
)
)

from tempfile import NamedTemporaryFile
import unittest

from scame.formatcheck import (
JavascriptChecker,
JS
)
)
from scame.tests import CheckerTestCase
from scame.tests.test_text import AnyTextMixin

Expand Down Expand Up @@ -73,9 +73,9 @@ def test_closure_linter(self):
checker = JavascriptChecker(
self.file.name, invalid_js, self.reporter)
checker.options.closure_linter.update({
'enabled': True,
'ignore': [],
})
'enabled': True,
'ignore': [],
})

checker.check()

Expand All @@ -91,9 +91,9 @@ def test_closure_linter_ignore(self):
checker = JavascriptChecker(
self.file.name, invalid_js, self.reporter)
checker.options.closure_linter.update({
'enabled': True,
'ignore': [2],
})
'enabled': True,
'ignore': [2],
})

checker.check()

Expand All @@ -105,9 +105,9 @@ def test_closure_linter_disabled(self):
checker = JavascriptChecker(
self.file.name, invalid_js, self.reporter)
checker.options.closure_linter.update({
'enabled': False,
'ignore': [],
})
'enabled': False,
'ignore': [],
})

checker.check()

Expand Down
2 changes: 1 addition & 1 deletion scame/tests/test_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
absolute_import,
print_function,
unicode_literals,
)
)

from scame.formatcheck import IS_PY3, JSONChecker
from scame.tests import CheckerTestCase
Expand Down
2 changes: 1 addition & 1 deletion scame/tests/test_reporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
absolute_import,
print_function,
unicode_literals,
)
)

from scame.tests import CheckerTestCase

Expand Down
62 changes: 58 additions & 4 deletions scame/tests/test_restructuredtext.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
absolute_import,
print_function,
unicode_literals,
)
)

from scame.formatcheck import ReStructuredTextChecker
from scame.tests import CheckerTestCase
Expand All @@ -23,14 +23,14 @@


--------------------
Second emtpy section
Second empty section
--------------------


Third section
^^^^^^^^^^^^^

Paragrhap for
Paragraph for
third section `with link<http://my.home>`_.

::
Expand All @@ -44,6 +44,10 @@
| verse, and adornment-free lists.


Newline test has newline.
Indeed.


.. _section-permalink:

Another Section with predefined link
Expand Down Expand Up @@ -141,7 +145,7 @@ def test_no_empty_last_line(self):
self.reporter.call_count = 0
content = (
'Some first line\n'
'the second and last line witout newline')
'the second and last line without newline')
checker = ReStructuredTextChecker('bogus', content, self.reporter)
checker.check_empty_last_line(2)
expected = [(
Expand Down Expand Up @@ -343,6 +347,56 @@ def test_check_section_delimiter_bad_marker_length(self):
self.assertEqual(expect, self.reporter.messages)
self.assertEqual(1, self.reporter.call_count)

def test_semantic_newline_alltests_true(self):
"""When a ., ?, or ! is not the last character of the line, it is
considered a semantic newline violation and an error is reported."""
content = (
'Sentence. \n'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test is wrong...as it will trigger the "trailing newspaces" checker

'Sentence! \n'
'Sentence? \n'
)
checker = ReStructuredTextChecker('bogus', content, self.reporter)
checker._check_semantic_newline()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the call with underline?

expect = ['Check that a new sentence is created after a full stop, ! or ?.']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the expected value is wrong. it should repot a tuple of line number and error message

self.assertEqual(expect, self.reporter.messages)
self.assertEqual(1, self.reporter.call_count)

def test_semantic_newline_fullstop(self):
"""When a full stop is not the last character of the line, it is
considered a semantic newline violation and an error is reported."""
content = (
'Sentence. New sentence\n'
)
checker = ReStructuredTextChecker('bogus', content, self.reporter)
checker._check_semantic_newline()
expect = ['Newline not created after a sentence.']
self.assertEqual(expect, self.reporter.messages)
self.assertEqual(1, self.reporter.call_count)

def test_semantic_newline_questionmark(self):
"""When a question mark is not the last character of the line, it is
considered a semantic newline violation and an error is reported."""
content = (
'Sentence? New sentence\n'
)
checker = ReStructuredTextChecker('bogus', content, self.reporter)
checker._check_semantic_newline()
expect = [('Newline not created after a ? sentence.')]
self.assertEqual(expect, self.reporter.messages)
self.assertEqual(1, self.reporter.call_count)

def test_semantic_newline_exclamationmark(self):
"""When an exclamation mark is not the last character of the line, it
is considered a semantic newline violation and an error is reported."""
content = (
'Sentence! New sentence\n'
)
checker = ReStructuredTextChecker('bogus', content, self.reporter)
checker._check_semantic_newline()
expect = [('Newline not created after a ! sentence.')]
self.assertEqual(expect, self.reporter.messages)
self.assertEqual(1, self.reporter.call_count)

def test_check_section_delimiter_bad_length_both_markers(self):
content = (
'---------\n'
Expand Down
2 changes: 1 addition & 1 deletion scame/tests/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
absolute_import,
print_function,
unicode_literals,
)
)

from scame.formatcheck import SQLChecker
from scame.tests import CheckerTestCase
Expand Down
Loading