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

Bears: Remove leading blank line option #2893

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
46 changes: 45 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,
indent_size: int = SpacingHelper.DEFAULT_TAB_WIDTH,
enforce_newline_at_EOF: bool = True,
):
Expand All @@ -34,12 +35,55 @@ def run(self,
Number of spaces per indentation level.
:param enforce_newline_at_EOF:
Whether to enforce a newline at the End Of File.
:param allow_leading_blanklines:
Whether to allow leading blank lines at the start
of file or not.
'''
spacing_helper = SpacingHelper(indent_size)
result_texts = []
additional_info_texts = []

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

Choose a reason for hiding this comment

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

The function name needs to be more meaningful.

end_line = False
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
if replacement.strip() == '':
end_line = line_number
else:
break
return end_line
Copy link
Member

Choose a reason for hiding this comment

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

In your case, end_line is either False or a number. I suggest not doing this.
I would have 2 functions, has_leading_blanklines() and get_leading_blanklines_no() or similar.


if allow_leading_blanklines:
start_line_of_file = 1
else:
end_blanklines = end_blanklines()
start_line_of_file = 1
if end_blanklines:
start_line_of_file = end_blanklines + 1
result_texts.append('Leading blank lines.')
additional_info_texts.append(
'Your source code contains leading blank lines.'
'Those usually have no meaning. Please consider '
'removing them.')
diff = Diff(file)
diff.delete_lines(1, end_blanklines)
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
24 changes: 24 additions & 0 deletions tests/general/SpaceConsistencyBearTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,28 @@ 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 +77,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',
Copy link
Member

Choose a reason for hiding this comment

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

From this test case, I cannot see whether the invalidity is due to a blank line (first line), or trailing white spaces (second line).

Copy link
Member

Choose a reason for hiding this comment

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

You should also verify the error message.

'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.

This file shouldn't a part of this PR.

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

Expand Down