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

SpaceConsistencyBear: Add leading_blankline option #2222

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
49 changes: 48 additions & 1 deletion bears/general/SpaceConsistencyBear.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def run(self,
file,
use_spaces: bool,
allow_trailing_whitespace: bool=False,
allow_leading_blanklines: bool=False,
Copy link
Member

Choose a reason for hiding this comment

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

add spaces around = ; new style requirement.

indent_size: int=SpacingHelper.DEFAULT_TAB_WIDTH,
enforce_newline_at_EOF: bool=True):
'''
Expand All @@ -29,6 +30,9 @@ def run(self,
True if spaces are to be used instead of tabs.
:param allow_trailing_whitespace:
Whether to allow trailing whitespace or not.
:param allow_leading_blanklines:
Whether to allow leading blanklines
Copy link
Contributor

Choose a reason for hiding this comment

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

blank lines not blanklines

at start of file or not.
Copy link
Contributor

Choose a reason for hiding this comment

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

at the start not at start

:param indent_size:
Number of spaces per indentation level.
:param enforce_newline_at_EOF:
Expand All @@ -38,7 +42,50 @@ def run(self,
result_texts = []
additional_info_texts = []

for line_number, line in enumerate(file, start=1):
def get_blanklines_nr_end():
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we come up with a better name for the function? This name is quite misleading.

line_nr_end = False
Copy link
Contributor

Choose a reason for hiding this comment

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

Again a better name for the variable.

enumerated_zip_obj = zip(range(1, len(file) + 1),
file)
enumerated_tuple = tuple(enumerated_zip_obj)

for line_number, line in enumerated_tuple:
replacement = line
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need for a temporary variable. Python uses Call by Object Reference and not Pass by reference.

if replacement.strip() == '':
line_nr_end = line_number
else:
break

return line_nr_end

if allow_leading_blanklines:
start_line_of_file = 1

else:
blanklines_nr_end = get_blanklines_nr_end()
start_line_of_file = 1
if blanklines_nr_end:
start_line_of_file = blanklines_nr_end + 1
result_texts.append('Leading blanklines.')
additional_info_texts.append(
'Your source code contains leading blanklines.'
Copy link
Contributor

Choose a reason for hiding this comment

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

Again blank lines

'Those usually have no meaning. Please consider'
Copy link
Contributor

Choose a reason for hiding this comment

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

You missed a space after Please consider

'removing them.')
diff = Diff(file)
diff.delete_lines(1, blanklines_nr_end)
inconsistencies = ''.join('\n- ' + string
for string in result_texts)
yield Result.from_values(
self,
'Line contains following spacing inconsistencies:'
+ inconsistencies,
diffs={filename: diff},
file=filename,
additional_info='\n\n'.join(additional_info_texts))
result_texts = []
additional_info_texts = []

for line_number, line in enumerate(file[start_line_of_file - 1:],
start=start_line_of_file):
replacement = line

if enforce_newline_at_EOF:
Expand Down
1 change: 0 additions & 1 deletion tests/c_languages/ClangBearTest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

from bears.c_languages.ClangBear import ClangBear
Copy link
Contributor

Choose a reason for hiding this comment

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

This as well ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

@newbazz as I told you before :) this patch was shown in CI just because of feature that I have added ;)
you can see I deleted leading blanklines from these files.

from coalib.testing.LocalBearTestHelper import verify_local_bear

Expand Down
25 changes: 24 additions & 1 deletion tests/general/SpaceConsistencyBearTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,27 @@ def test_data_sets_tabs(self):
self.section.append(Setting('use_spaces', 'false'))
self.section.append(Setting('allow_trailing_whitespace', 'true'))
self.section.append(Setting('enforce_newline_at_EOF', 'false'))
self.section.append(Setting('allow_leading_blanklines', 'false'))

self.check_invalidity(self.uut, [' t'])
self.check_validity(self.uut, ['t \n'])
self.check_validity(self.uut, ['\tt\n'])
self.check_validity(self.uut, [])

def test_enforce_newline_at_eof(self):
self.section.append(Setting('use_spaces', 'true'))
self.section.append(Setting('allow_trailing_whitespace', 'true'))
self.section.append(Setting('enforce_newline_at_EOF', 'true'))
self.section.append(Setting('allow_leading_blanklines', 'true'))

self.check_validity(self.uut,
['hello world \n'],
force_linebreaks=False)
self.check_validity(self.uut,
['def somecode():\n',
[' \n',
'\n',
' \n',
'def somecode():\n',
" print('funny')\n",
" print('funny end.')\n"],
force_linebreaks=False)
Expand All @@ -70,3 +76,20 @@ def test_enforce_newline_at_eof(self):
" print('funny')\n",
" print('the result is not funny...')"],
force_linebreaks=False)

def test_leading_blanklines(self):
self.section.append(Setting('use_spaces', 'true'))
self.section.append(Setting('allow_trailing_whitespace', 'false'))
self.section.append(Setting('enforce_newline_at_EOF', 'true'))
self.section.append(Setting('allow_leading_blanklines', 'false'))

self.check_invalidity(self.uut,
['\n',
' \n',
'def code():\n',
" print('Am I coding?')\n"],
force_linebreaks=False)
self.check_validity(self.uut,
['def code():\n',
" print('Am I coding?')\n"],
force_linebreaks=False)
1 change: 0 additions & 1 deletion tests/go/GoImportsBearTest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change? I think you sud file a new issue for this or just include this in a different commit

from bears.go.GoImportsBear import GoImportsBear
from coalib.testing.LocalBearTestHelper import verify_local_bear

Expand Down