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

aspectsYEAHBear.py: Add aspectsYEAHBear #1604

Closed
wants to merge 1 commit into from
Closed
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
67 changes: 67 additions & 0 deletions bears/general/aspectsYEAHBear.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import re

from coalib.bears.LocalBear import LocalBear
from coalib.results.Diff import Diff
from coalib.results.Result import Result
from coalib.results.RESULT_SEVERITY import RESULT_SEVERITY
from coalib.bearlib.aspects.Spelling import aspectsYEAH
from coalib.bearlib.languages import Unknown


class aspectsYEAHBear(LocalBear, aspects={
'detect': [aspectsYEAH],
'fix': [aspectsYEAH], }):
LANGUAGES = {'All'}
AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'[email protected]'}
LICENSE = 'AGPL-3.0'
CAN_DETECT = {'Spelling'}

def run(self, filename, file, aspects=[
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this aspects in the run function?

Copy link
Member

Choose a reason for hiding this comment

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

Do you wish to run this bear via passing the aspects or something like that?

Copy link
Member

Choose a reason for hiding this comment

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

Whatever it is please explain param aspects in the docstring.

Copy link
Member

Choose a reason for hiding this comment

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

If you are willing to call it via aspects, the default value has to be Root perhaps instead of aspectsYEAH. Well we need to make it universal for all the bears therefore modify it in the run method of Bear class. Remember the discussion we had for the default value. I meant to place it here actually and by mistake I triggered a discussion in the instantiation of Bear class.

aspectsYEAH(Unknown),
]):
"""
Check for the correct spelling of ``aspect`` and ``aspectsYEAH``
in the file.
"""

for aspect in aspects:
if type(aspect) is aspectsYEAH:
Copy link
Member

Choose a reason for hiding this comment

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

type(aspect) is <aspectclass 'Root.Spelling.aspectsYEAH'> not aspectsYEAH

Copy link
Member

@Techievena Techievena Apr 12, 2017

Choose a reason for hiding this comment

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

Better replace with if type(aspect).__name__ is 'aspectsYEAH':. 😃

Copy link
Member

Choose a reason for hiding this comment

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

@Techievena If aspect is an instance of aspectsYEAH then type(aspect) is the same class object as referenced by aspectsYEAH. "<aspectclass 'Root.Spelling.aspectsYEAH'>" is the repr(aspectsYEAH). So nothing is wrong with that condition.

Copy link
Member

Choose a reason for hiding this comment

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

We only need to implement a more generalized way of running bear logic for supported aspects than in such a for loop with several if type(aspect) is ... checks

Copy link
Member

Choose a reason for hiding this comment

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

My bad must have checked properly before reviewing.

corrected_aspects = []
corrected_aspectsYEAH = []
for line in file:
wrong_spelling1 = r'(?!aspect)[aA][sS][pP][eE][cC][tT]'
Copy link
Member

Choose a reason for hiding this comment

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

You only check for 'aspect' but not for 'aspects' here

wrong_spelling2 = r'(?!aspectsYEAH)[aA][sS]'
'[pP][eE][cC][tT][sS][yY][eE][aA][hH]'
if not re.match(wrong_spelling2, line):
corrected_aspects += [re.sub(wrong_spelling1,
'aspects',
Copy link
Member

Choose a reason for hiding this comment

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

and here you replace all misspelled 'aspect' with 'aspects'

line)]
corrected_aspectsYEAH += [re.sub(wrong_spelling2,
'aspectsYEAH',
line)]

aspects_diffs = Diff.from_string_arrays(file,
corrected_aspects
).split_diff()
aspectsYEAH_diffs = \
Diff.from_string_arrays(file,
corrected_aspectsYEAH
).split_diff()
for diff in aspects_diffs:
yield Result(self,
'``aspect`` is always written with'
' lower case characters',
aspect=aspect,
affected_code=(diff.range(filename),),
diffs={filename: diff},
severity=RESULT_SEVERITY.MAJOR)
for diff in aspectsYEAH_diffs:
yield Result(self,
'``aspectsYEAH`` is always written with'
' lower case ``aspects`` and upper case '
'``YEAH``',
aspect=aspect,
affected_code=(diff.range(filename),),
diffs={filename: diff},
severity=RESULT_SEVERITY.MAJOR)
42 changes: 42 additions & 0 deletions tests/general/aspectsYEAHBearTest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import unittest
from queue import Queue

from coalib.settings.Section import Section
from coalib.results.RESULT_SEVERITY import RESULT_SEVERITY
from coalib.bearlib.aspects.Spelling import aspectsYEAH
from bears.general.aspectsYEAHBear import aspectsYEAHBear


class aspectsYEAHBearTest(unittest.TestCase):

def setUp(self):
self.section = Section('name')
self.queue = Queue()
self.uut = aspectsYEAHBear(self.section, self.queue)

def test_severity(self):
bad_file = """
AsPectYEAH
"""
results = list(self.uut.run('bad_file', bad_file.split('\n')))
self.assertEqual(results[0].severity,
RESULT_SEVERITY.MAJOR)

def test_message(self):
bad_file = """
Aspects
Aspectyeah"""
results = list(self.uut.run('bad_file', bad_file.split('\n')))
self.assertEqual(results[0].message,
'``aspect`` is always written'
Copy link
Member

Choose a reason for hiding this comment

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

``aspect`` and ``aspects`` are always...

' with lower case characters')
self.assertEqual(results[1].message,
'``aspectsYEAH`` is always written with '
'lower case ``aspects`` and upper case ``YEAH``')

def test_aspect(self):
bad_file = """
Aspects
"""
results = list(self.uut.run('bad_file', bad_file.split('\n')))
self.assertEqual(type(results[0].aspect), aspectsYEAH)