VultureBear: Make use of vulture API #2000

RJ722 · 2017-08-15T09:15:38Z

Checklist

I read the commit guidelines and I've followed
them.
I ran coala over my code locally. (All commits have to pass
individually. It is not sufficient to have "fixup commits" on your PR,
our bot will still report the issues for the previous commit.) You will
likely receive a lot of bot comments and build failures if coala does not
pass on every single commit!

Please consider helping us by reviewing other peoples pull requests as well:

pick up any PR at https://coala.io/review
review it (check https://coala.io/reviewing for more info)
if you are sure that it needs work, use cobot mark wip <URL> to get it out
of the review queue.

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

gitmate-bot · 2017-08-15T20:19:43Z

tests/python/VultureBearTest.py

+                                        item.affected_code[0].end.line,
+                                        item.confidence))
+                        for item in self.get_results(test_case))
+        self.assertEqual(detected, expected)        


W291 trailing whitespace'

PycodestyleBear (W291), severity NORMAL, section autopep8.

gitmate-bot · 2017-08-15T20:19:44Z

tests/python/VultureBearTest.py

@@ -41,86 +41,117 @@ def prep_file():
                stack.enter_context(prep_file())

            return list(self.uut.run())
+


W293 blank line contains whitespace'

PycodestyleBear (W293), severity NORMAL, section autopep8.

gitmate-bot · 2017-08-15T20:19:45Z

tests/python/VultureBearTest.py

+                                        item.affected_code[0].end.line,
+                                        item.confidence))
+                        for item in self.get_results(test_case))
+        self.assertEqual(detected, expected)        


The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/tests/python/VultureBearTest.py +++ b/tests/python/VultureBearTest.py @@ -47,7 +47,7 @@ item.affected_code[0].end.line, item.confidence)) for item in self.get_results(test_case)) - self.assertEqual(detected, expected) + self.assertEqual(detected, expected) def test_used_variable(self): good_file = """\

gitmate-bot · 2017-08-15T20:19:46Z

tests/python/VultureBearTest.py

@@ -41,86 +41,117 @@ def prep_file():
                stack.enter_context(prep_file())

            return list(self.uut.run())
+


The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/tests/python/VultureBearTest.py +++ b/tests/python/VultureBearTest.py @@ -41,7 +41,7 @@ stack.enter_context(prep_file()) return list(self.uut.run()) - + def verify_results(self, test_case, expected): detected = dict((item.message, (item.affected_code[0].start.line, item.affected_code[0].end.line,

gitmate-bot · 2017-08-15T20:19:47Z

tests/python/VultureBearTest.py

+                                        item.affected_code[0].end.line,
+                                        item.confidence))
+                        for item in self.get_results(test_case))
+        self.assertEqual(detected, expected)        


Line contains following spacing inconsistencies:

Trailing whitespaces.

SpaceConsistencyBear, severity NORMAL, section python.

The issue can be fixed by applying the following patch:

--- a/tests/python/VultureBearTest.py +++ b/tests/python/VultureBearTest.py @@ -47,7 +47,7 @@ item.affected_code[0].end.line, item.confidence)) for item in self.get_results(test_case)) - self.assertEqual(detected, expected) + self.assertEqual(detected, expected) def test_used_variable(self): good_file = """\

gitmate-bot · 2017-08-15T20:19:47Z

tests/python/VultureBearTest.py

@@ -41,86 +41,117 @@ def prep_file():
                stack.enter_context(prep_file())

            return list(self.uut.run())
+


Line contains following spacing inconsistencies:

Trailing whitespaces.

SpaceConsistencyBear, severity NORMAL, section python.

The issue can be fixed by applying the following patch:

--- a/tests/python/VultureBearTest.py +++ b/tests/python/VultureBearTest.py @@ -41,7 +41,7 @@ stack.enter_context(prep_file()) return list(self.uut.run()) - + def verify_results(self, test_case, expected): detected = dict((item.message, (item.affected_code[0].start.line, item.affected_code[0].end.line,

jendrikseipp · 2017-08-16T13:40:43Z

bear-requirements.txt

@@ -34,6 +34,6 @@ rstcheck~=3.1
 safety~=0.5.1
 scspell3k~=2.0
 vim-vint~=0.3.12
-vulture~=0.14.0
+vulture==0.24.0


Can we use 0.25? It should make integration even easier.

The new Item class is so cool. Native support for coala ;-)

jendrikseipp · 2017-08-16T13:46:25Z

tests/python/VultureBearTest.py

+    def test_unreachable_else(self):
+        bad_file = """\
+if True:
+    print("Reachanle")


jendrikseipp · 2017-08-16T13:47:18Z

tests/python/VultureBearTest.py

-        good_file = """
-        x = 2
-        print(x)
+        good_file = """\


instead of good_file and bad_file, can we just use code?

Yeah! That indeed looks irrelevant! 🤔

jendrikseipp · 2017-08-16T13:48:45Z

Congratulations on getting PR number 2000 :-)

Nosferatul · 2017-08-17T10:18:06Z

i think this looks good right now, what do you say @jendrikseipp ?

jendrikseipp · 2017-08-17T10:35:15Z

I'll have a look :-)

jendrikseipp · 2017-08-17T10:36:22Z

bears/python/VultureBear.py



 class VultureBear(GlobalBear):
    LANGUAGES = {'Python', 'Python 3'}
-    REQUIREMENTS = {PipRequirement('vulture', '0.14.0')}
+    REQUIREMENTS = {PipRequirement('vulture', '0.25.0')}


Is it normal that we have to list the requirement in two different places?

I remember doing it previously. FWIW I have asked in the gitter channel.

yeah currently we need it at two places, one for CI and one for the bear itself. Once our requirement-management gets more mature we will generate that dynamically as far as possible 👍

@jendrikseipp FYI the requirements.txt above is generated and the CI always checks that it's up to date so this REQUIREMENTS attribute is the correct and important place, the requirements.txt a technical necessity currently manually managed but at least enforced by the CI (last time I checked)

@sils Thanks for clarifying!

jendrikseipp · 2017-08-17T10:37:11Z

tests/python/VultureBearTest.py

-            print('Hello World')
+    def __init__(self, name):
+        self.name = name
+        self.sur_name = 'Something'


sur_name -> surname

jendrikseipp · 2017-08-17T10:37:33Z

tests/python/VultureBearTest.py

+    def test_unreachable_else(self):
+        code = """\
+if True:
+    print("Reachanle")


jendrikseipp · 2017-08-17T10:41:31Z

tests/python/VultureBearTest.py

-        class Name:
+    def test_class(self):
+        code = """\
+class Name:



Can you please remove this empty line?

jendrikseipp · 2017-08-17T10:42:42Z

bears/python/VultureBear.py

-            vulture.unused_props + vulture.unused_vars +
-            vulture.unused_attrs, key=file_lineno):
-        message = 'Unused {0}: {1}'.format(item.typ, item)
+    for item in vulture.get_unused_code(sort_by_size=True):


Is sorting helpful here? I would guess that coala has its own sorting. And if it doesn't, I think we should sort by filenames.

I think coala doesn't sorts the results at all because results are yielded at runtime directly, without being stored (generally).

Also, since files would be automatically be coming in an alphabetical order, I think we won't need additional sorting. I am removing the sort_by_size=True for now. Please comment if you think else wise. :-)

jendrikseipp · 2017-08-17T10:43:48Z

bears/python/VultureBear.py

-                                 confidence=CONFIDENCE_MAP[item.typ])
+                                 line=item.first_lineno,
+                                 end_line=item.last_lineno,
+                                 confidence=item.confidence)


Is it already possible to pass a minimum confidence to coala? If not, we should create an issue for this.

I don't know!

CC @jayvdb

we plan to support this, I think there should be somewhere an issue 👍 (can't find it right now)

otherwise feel free to file one ;)

jendrikseipp · 2017-08-17T10:44:21Z

bears/python/VultureBear.py

-        return (item.filename.lower(), item.lineno)
-
-    vulture = Vulture()
+    vulture = Vulture(verbose=True)


I don't think we should be verbose.

jayvdb · 2017-08-26T07:28:01Z

Interesting. Have you checked if there is a new version of alex which has new rules preventing 'his' ?

RJ722 · 2017-08-26T16:29:26Z

@jayvdb I don't think that there were any upstream changes, https://github.com/azu/textlint-rule-alex/ was last updated - Nov, 16.

Should we try rebuilding without cache?

jendrikseipp · 2017-08-28T06:34:11Z

tests/python/VultureBearTest.py

-        result = self.get_results(bad_file)
-        self.assertEqual(len(result), 1)
-        self.assertEqual(result[0].confidence, 70)
+        code = load_testfile('unused_variable.py')


I'd pass the filename to self.verify_results().

Thanks for the suggestion 😄 Done!

jendrikseipp · 2017-08-28T07:36:56Z

Looks good to me :-)

jendrikseipp · 2017-08-28T13:08:55Z

It seems the tests are still failing. Is there something we can do about that?

Makman2 · 2017-08-28T13:46:37Z

phew CI is going crazy... try a rebase, we recently merged some fix for CI.

Makman2 · 2017-08-28T13:48:50Z

but yeah commit looks also fine to me :)

jendrikseipp · 2017-08-28T20:22:43Z

@RJ722 Does rebasing help with the tests?

The new vulture API provides better integration with coala. It natively supports `item.confidence`. Also, it gives precise information about the location of dead code (`first_line, `last_line`)

RJ722 · 2017-08-29T02:41:01Z

@RJ722 Does rebasing help with the tests?

Done rebasing. Praying starts. 🙏

jendrikseipp · 2017-08-29T06:46:32Z

@RJ722 Tests passed :-)

@Makman2 can we merge this?

Makman2 · 2017-08-29T13:56:15Z

reack 58a3999

Makman2 · 2017-08-29T13:56:45Z

@rultor merge

rultor · 2017-08-29T13:58:03Z

@rultor merge

@Makman2 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2017-08-29T13:59:52Z

@rultor merge

@Makman2 Done! FYI, the full log is here (took me 2min)

Makman2 · 2017-08-29T14:00:26Z

yeah 💥 tough PR, but we got it :D

RJ722 · 2017-08-30T16:17:06Z

yeah 💥 tough PR, but we got it :D

@Makman2 Roger that!

BTW I've opened #2025

gitmate-bot added process/pending review size/S labels Aug 15, 2017

coala-bot added process/wip and removed process/pending review labels Aug 15, 2017

RJ722 force-pushed the use-vulture-API branch from 2bf4cce to d32bcef Compare August 15, 2017 09:36

gitmate-bot added process/pending review and removed process/wip labels Aug 15, 2017

RJ722 force-pushed the use-vulture-API branch from d32bcef to cd11eb7 Compare August 15, 2017 20:12

gitmate-bot added size/L and removed size/S labels Aug 15, 2017

gitmate-bot reviewed Aug 15, 2017

View reviewed changes

RJ722 force-pushed the use-vulture-API branch 2 times, most recently from ebe94e9 to 7a23b1b Compare August 16, 2017 13:01

jendrikseipp suggested changes Aug 16, 2017

View reviewed changes

RJ722 force-pushed the use-vulture-API branch 4 times, most recently from adf4385 to 2a1d0c1 Compare August 16, 2017 16:43

jendrikseipp suggested changes Aug 17, 2017

View reviewed changes

RJ722 force-pushed the use-vulture-API branch 3 times, most recently from 968107c to 5ddd09e Compare August 18, 2017 14:31

RJ722 force-pushed the use-vulture-API branch 4 times, most recently from 566dcb9 to 5378955 Compare August 28, 2017 06:04

jendrikseipp reviewed Aug 28, 2017

View reviewed changes

RJ722 force-pushed the use-vulture-API branch from 5378955 to 54d944a Compare August 28, 2017 07:31

VultureBear: Make use of the new vulture API

58a3999

The new vulture API provides better integration with coala. It natively supports `item.confidence`. Also, it gives precise information about the location of dead code (`first_line, `last_line`)

RJ722 force-pushed the use-vulture-API branch from 54d944a to 58a3999 Compare August 29, 2017 02:36

Makman2 added process/approved and removed process/pending review labels Aug 29, 2017

rultor merged commit 58a3999 into coala:master Aug 29, 2017

RJ722 mentioned this pull request Aug 30, 2017

RJ722: Update report coala/projects#361

Merged

2 tasks

RJ722 deleted the use-vulture-API branch August 30, 2017 16:11

twmr mentioned this pull request Sep 30, 2017

use vulture to automatically remove dead code? jendrikseipp/vulture#25

Closed

graingert mentioned this pull request Aug 19, 2019

Rewrite six.PY2 ternary asottile/pyupgrade#162

Open

		@@ -41,86 +41,117 @@ def prep_file():
		stack.enter_context(prep_file())

		return list(self.uut.run())

VultureBear: Make use of vulture API #2000

VultureBear: Make use of vulture API #2000

Conversation

RJ722 commented Aug 15, 2017

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jendrikseipp commented Aug 16, 2017

Nosferatul commented Aug 17, 2017

jendrikseipp commented Aug 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayvdb commented Aug 26, 2017

RJ722 commented Aug 26, 2017 • edited Loading

Choose a reason for hiding this comment

RJ722 Aug 28, 2017 • edited Loading

Choose a reason for hiding this comment

jendrikseipp commented Aug 28, 2017

jendrikseipp commented Aug 28, 2017

Makman2 commented Aug 28, 2017

Makman2 commented Aug 28, 2017 • edited Loading

jendrikseipp commented Aug 28, 2017

RJ722 commented Aug 29, 2017

jendrikseipp commented Aug 29, 2017

Makman2 commented Aug 29, 2017

Makman2 commented Aug 29, 2017

rultor commented Aug 29, 2017

rultor commented Aug 29, 2017

Makman2 commented Aug 29, 2017

RJ722 commented Aug 30, 2017

RJ722 commented Aug 26, 2017 •

edited

Loading

RJ722 Aug 28, 2017 •

edited

Loading

Makman2 commented Aug 28, 2017 •

edited

Loading