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

CPPLintBear.py: Verify indentation and spaces #2105

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
13 changes: 12 additions & 1 deletion bears/c_languages/CPPLintBear.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
from coalib.settings.Setting import typed_list


def check_invalid_configuration(indent_size, use_spaces):
if indent_size != 2 or not use_spaces:
raise ValueError('CPPLint only supports an indentation size of 2, '
'using only spaces.')


@linter(executable='cpplint',
use_stdout=False,
use_stderr=True,
Expand All @@ -26,12 +32,17 @@ class CPPLintBear:
def create_arguments(filename, file, config_file,
max_line_length: int=79,
cpplint_ignore: typed_list(str)=(),
cpplint_include: typed_list(str)=()):
cpplint_include: typed_list(str)=(),
indent_size: int=2,
Copy link
Member

Choose a reason for hiding this comment

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

Do we want these as defaults or do we want the user to fail without any given argument. With a default value, users might think 4 just works. But I guess they should read the docs and then they'd see.

Copy link
Member Author

Choose a reason for hiding this comment

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

Case A: Fails without any given argument. Then maybe they set this argument to 4, which fails again. Then they read the docs and set it to 2.
Case B: Works by default. If user wants to override, docs clearly call out the permitted value.
Thoughts?

use_spaces: bool=True):
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any other way to check what kind of indent_size the user is using than relying on the user himself to give that information? Also does this fix it in case the indent_size is not matching or the user is using tabs?

:param max_line_length: Maximum number of characters for a line.
:param cpplint_ignore: List of checkers to ignore.
:param cpplint_include: List of checkers to explicitly enable.
:param indent_size: Only an indent size of 2 is permitted.
:param use_spaces: Only the value `True` is permitted.
"""
check_invalid_configuration(indent_size, use_spaces)
ignore = ','.join('-'+part.strip() for part in cpplint_ignore)
include = ','.join('+'+part.strip() for part in cpplint_include)
return ('--filter=' + ignore + ',' + include,
Expand Down
29 changes: 28 additions & 1 deletion tests/c_languages/CPPLintBearTest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
from queue import Queue

from bears.c_languages.CPPLintBear import CPPLintBear
from coalib.testing.LocalBearTestHelper import verify_local_bear
from coalib.testing.LocalBearTestHelper import (verify_local_bear,
LocalBearTestHelper)
from coalib.testing.BearTestHelper import generate_skip_decorator
from coalib.settings.Section import Section
from coalib.settings.Setting import Setting


test_file = """
int main() {
Expand All @@ -26,3 +33,23 @@
settings={'cpplint_ignore': 'legal',
'max_line_length': '13'},
tempfile_kwargs={'suffix': '.cpp'})


@generate_skip_decorator(CPPLintBear)
class CPPLintBearTest(LocalBearTestHelper):

CPP_VALUE_ERROR_RE = 'Bear returned None on execution'

def setUp(self):
self.section = Section('test section')
self.uut = CPPLintBear(self.section, Queue())

def test_config_failure_indent_size(self):
self.section.append(Setting('indent_size', 3))
with self.assertRaisesRegex(AssertionError, self.CPP_VALUE_ERROR_RE):
self.check_validity(self.uut, [], test_file)

def test_config_failure_use_spaces(self):
self.section.append(Setting('use_spaces', False))
with self.assertRaisesRegex(AssertionError, self.CPP_VALUE_ERROR_RE):
self.check_validity(self.uut, [], test_file)