From 9eb9853d52091a24ceffd04679762a7efedc98fd Mon Sep 17 00:00:00 2001 From: Michal Nowikowski Date: Fri, 1 Aug 2014 06:30:10 +0200 Subject: Improved presenting unused-import message. Closes issue #293. --- checkers/variables.py | 16 ++++++++-------- test/functional/unused_import.py | 3 +++ test/functional/unused_import.txt | 2 ++ 3 files changed, 13 insertions(+), 8 deletions(-) create mode 100644 test/functional/unused_import.py create mode 100644 test/functional/unused_import.txt diff --git a/checkers/variables.py b/checkers/variables.py index 338615f..e1ae0ec 100644 --- a/checkers/variables.py +++ b/checkers/variables.py @@ -319,14 +319,14 @@ builtins. Remember that you should avoid to define new builtins when possible.' and isinstance(stmt.ass_type(), astroid.AugAssign) for stmt in stmts): continue - stmt = stmts[0] - if isinstance(stmt, astroid.Import): - self.add_message('unused-import', args=name, node=stmt) - elif isinstance(stmt, astroid.From) and stmt.modname != '__future__': - if stmt.names[0][0] == '*': - self.add_message('unused-wildcard-import', args=name, node=stmt) - else: - self.add_message('unused-import', args=name, node=stmt) + for stmt in stmts: + if isinstance(stmt, astroid.Import): + self.add_message('unused-import', args=stmt.names[0][0], node=stmt) + elif isinstance(stmt, astroid.From) and stmt.modname != '__future__': + if stmt.names[0][0] == '*': + self.add_message('unused-wildcard-import', args=name, node=stmt) + else: + self.add_message('unused-import', args=name, node=stmt) del self._to_consume def visit_class(self, node): diff --git a/test/functional/unused_import.py b/test/functional/unused_import.py new file mode 100644 index 0000000..c4bc63c --- /dev/null +++ b/test/functional/unused_import.py @@ -0,0 +1,3 @@ +"""unused import""" +import xml.etree # [unused-import] +import xml.sax # [unused-import] diff --git a/test/functional/unused_import.txt b/test/functional/unused_import.txt new file mode 100644 index 0000000..7b6e363 --- /dev/null +++ b/test/functional/unused_import.txt @@ -0,0 +1,2 @@ +unused-import:2::Unused import xml.etree +unused-import:3::Unused import xml.sax -- cgit v1.2.1 From 29df6196ec16b8ad935de883d20985bd8c08684d Mon Sep 17 00:00:00 2001 From: Michal Nowikowski Date: Fri, 1 Aug 2014 07:12:30 +0200 Subject: Added entry to changelog about closing issue 293. --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index d3e8d82..f8af093 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,7 @@ ChangeLog for Pylint ==================== -- + * Improved presenting unused-import message. Closes issue #293. * Added new checks for line endings if they are mixed (LF vs CRLF) or if they are not as expected. New messages: mixed-line-endings, -- cgit v1.2.1 From 067b083930d7aa751ac8d5698e90e49adeb63b2e Mon Sep 17 00:00:00 2001 From: Michal Nowikowski Date: Sat, 2 Aug 2014 06:18:30 +0200 Subject: Improved messages text for unused imports. --- checkers/variables.py | 14 +++++++++++--- test/functional/unused_import.py | 2 ++ test/functional/unused_import.txt | 2 ++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/checkers/variables.py b/checkers/variables.py index e1ae0ec..3893a4a 100644 --- a/checkers/variables.py +++ b/checkers/variables.py @@ -170,7 +170,7 @@ MSGS = { 'global-at-module-level', 'Used when you use the "global" statement at the module level \ since it has no effect'), - 'W0611': ('Unused import %s', + 'W0611': ('Unused %s', 'unused-import', 'Used when an imported module or variable is not used.'), 'W0612': ('Unused variable %r', @@ -321,12 +321,20 @@ builtins. Remember that you should avoid to define new builtins when possible.' continue for stmt in stmts: if isinstance(stmt, astroid.Import): - self.add_message('unused-import', args=stmt.names[0][0], node=stmt) + if stmt.names[0][1] is None: + msg = "import %s" % stmt.names[0][0] + else: + msg = "%s imported as %s" % (stmt.names[0][0], stmt.names[0][1]) + self.add_message('unused-import', args=msg, node=stmt) elif isinstance(stmt, astroid.From) and stmt.modname != '__future__': if stmt.names[0][0] == '*': self.add_message('unused-wildcard-import', args=name, node=stmt) else: - self.add_message('unused-import', args=name, node=stmt) + if stmt.names[0][1] is None: + msg = "%s imported from %s" % stmt.names[0][0], stmt.modname + else: + msg = "%s imported from %s as %s" % (stmt.names[0][0], stmt.modname, stmt.names[0][1]) + self.add_message('unused-import', args=msg, node=stmt) del self._to_consume def visit_class(self, node): diff --git a/test/functional/unused_import.py b/test/functional/unused_import.py index c4bc63c..e129a41 100644 --- a/test/functional/unused_import.py +++ b/test/functional/unused_import.py @@ -1,3 +1,5 @@ """unused import""" import xml.etree # [unused-import] import xml.sax # [unused-import] +import os.path as test # [unused-import] +from sys import argv as test2 # [unused-import] diff --git a/test/functional/unused_import.txt b/test/functional/unused_import.txt index 7b6e363..627fb2d 100644 --- a/test/functional/unused_import.txt +++ b/test/functional/unused_import.txt @@ -1,2 +1,4 @@ unused-import:2::Unused import xml.etree unused-import:3::Unused import xml.sax +unused-import:4::Unused os.path imported as test +unused-import:5::Unused argv imported from sys as test2 -- cgit v1.2.1 From 8ef5d79f2f65469ba08c3e5ef4f79be010e389da Mon Sep 17 00:00:00 2001 From: Michal Nowikowski Date: Sat, 2 Aug 2014 06:31:13 +0200 Subject: Fixed preparing unused-import message. --- checkers/variables.py | 2 +- test/functional/unused_import.py | 1 + test/functional/unused_import.txt | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/checkers/variables.py b/checkers/variables.py index 3893a4a..68dd0d5 100644 --- a/checkers/variables.py +++ b/checkers/variables.py @@ -331,7 +331,7 @@ builtins. Remember that you should avoid to define new builtins when possible.' self.add_message('unused-wildcard-import', args=name, node=stmt) else: if stmt.names[0][1] is None: - msg = "%s imported from %s" % stmt.names[0][0], stmt.modname + msg = "%s imported from %s" % (stmt.names[0][0], stmt.modname) else: msg = "%s imported from %s as %s" % (stmt.names[0][0], stmt.modname, stmt.names[0][1]) self.add_message('unused-import', args=msg, node=stmt) diff --git a/test/functional/unused_import.py b/test/functional/unused_import.py index e129a41..08aa0ee 100644 --- a/test/functional/unused_import.py +++ b/test/functional/unused_import.py @@ -3,3 +3,4 @@ import xml.etree # [unused-import] import xml.sax # [unused-import] import os.path as test # [unused-import] from sys import argv as test2 # [unused-import] +from sys import flags # [unused-import] diff --git a/test/functional/unused_import.txt b/test/functional/unused_import.txt index 627fb2d..53243c1 100644 --- a/test/functional/unused_import.txt +++ b/test/functional/unused_import.txt @@ -2,3 +2,4 @@ unused-import:2::Unused import xml.etree unused-import:3::Unused import xml.sax unused-import:4::Unused os.path imported as test unused-import:5::Unused argv imported from sys as test2 +unused-import:6::Unused flags imported from sys -- cgit v1.2.1 From 813773747653ee85ea4d52bf9cb3339f42833844 Mon Sep 17 00:00:00 2001 From: Michal Nowikowski Date: Mon, 4 Aug 2014 06:41:14 +0200 Subject: Review fixes. --- checkers/variables.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/checkers/variables.py b/checkers/variables.py index 68dd0d5..b2f27de 100644 --- a/checkers/variables.py +++ b/checkers/variables.py @@ -320,20 +320,26 @@ builtins. Remember that you should avoid to define new builtins when possible.' for stmt in stmts): continue for stmt in stmts: + if not isinstance(stmt, astroid.Import) and not isinstance(stmt, astroid.From): + continue + + imported_name = stmt.names[0][0] # this is: 'import imported_name' or 'from something import imported_name' + as_name = stmt.names[0][1] # this is: 'import imported_name as as_name' + if isinstance(stmt, astroid.Import): - if stmt.names[0][1] is None: - msg = "import %s" % stmt.names[0][0] + if as_name is None: + msg = "import %s" % imported_name else: - msg = "%s imported as %s" % (stmt.names[0][0], stmt.names[0][1]) + msg = "%s imported as %s" % (imported_name, as_name) self.add_message('unused-import', args=msg, node=stmt) elif isinstance(stmt, astroid.From) and stmt.modname != '__future__': - if stmt.names[0][0] == '*': + if imported_name == '*': self.add_message('unused-wildcard-import', args=name, node=stmt) else: - if stmt.names[0][1] is None: - msg = "%s imported from %s" % (stmt.names[0][0], stmt.modname) + if as_name is None: + msg = "%s imported from %s" % (imported_name, stmt.modname) else: - msg = "%s imported from %s as %s" % (stmt.names[0][0], stmt.modname, stmt.names[0][1]) + msg = "%s imported from %s as %s" % (imported_name, stmt.modname, as_name) self.add_message('unused-import', args=msg, node=stmt) del self._to_consume -- cgit v1.2.1 From f045d560a4551bbd89838bb589db0753f90b1699 Mon Sep 17 00:00:00 2001 From: Michal Nowikowski Date: Mon, 4 Aug 2014 17:27:35 +0200 Subject: Fix for wired case with import from stmt. --- checkers/variables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkers/variables.py b/checkers/variables.py index b2f27de..9f1c52f 100644 --- a/checkers/variables.py +++ b/checkers/variables.py @@ -326,7 +326,7 @@ builtins. Remember that you should avoid to define new builtins when possible.' imported_name = stmt.names[0][0] # this is: 'import imported_name' or 'from something import imported_name' as_name = stmt.names[0][1] # this is: 'import imported_name as as_name' - if isinstance(stmt, astroid.Import): + if isinstance(stmt, astroid.Import) or (isinstance(stmt, astroid.From) and not stmt.modname): if as_name is None: msg = "import %s" % imported_name else: -- cgit v1.2.1 From ec317953df3176260f51df5bca42d576c320af33 Mon Sep 17 00:00:00 2001 From: Michal Nowikowski Date: Mon, 4 Aug 2014 17:34:58 +0200 Subject: Improved comments. --- checkers/variables.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/checkers/variables.py b/checkers/variables.py index 9f1c52f..85349e6 100644 --- a/checkers/variables.py +++ b/checkers/variables.py @@ -323,8 +323,10 @@ builtins. Remember that you should avoid to define new builtins when possible.' if not isinstance(stmt, astroid.Import) and not isinstance(stmt, astroid.From): continue - imported_name = stmt.names[0][0] # this is: 'import imported_name' or 'from something import imported_name' - as_name = stmt.names[0][1] # this is: 'import imported_name as as_name' + # 'import imported_name' or 'from something import imported_name' + imported_name = stmt.names[0][0] + # 'import imported_name as as_name' + as_name = stmt.names[0][1] if isinstance(stmt, astroid.Import) or (isinstance(stmt, astroid.From) and not stmt.modname): if as_name is None: -- cgit v1.2.1 From 1be680429bd86e93fe47c4398b82dccded80ea6d Mon Sep 17 00:00:00 2001 From: Michal Nowikowski Date: Wed, 6 Aug 2014 06:54:10 +0200 Subject: Fixed checking a list of imported names from a module in unused-import case. --- checkers/variables.py | 32 ++++++++++++++++++++++++++++---- test/functional/unused_import.py | 2 ++ test/functional/unused_import.txt | 3 +++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/checkers/variables.py b/checkers/variables.py index 85349e6..687e777 100644 --- a/checkers/variables.py +++ b/checkers/variables.py @@ -314,6 +314,10 @@ builtins. Remember that you should avoid to define new builtins when possible.' # don't check unused imports in __init__ files if not self.config.init_import and node.package: return + + # fix local names in node.locals dict (xml.etree instead of xml) + # TODO: this should be improved in issue astroid#46 + local_names = {} for name, stmts in not_consumed.iteritems(): if any(isinstance(stmt, astroid.AssName) and isinstance(stmt.ass_type(), astroid.AugAssign) @@ -322,13 +326,33 @@ builtins. Remember that you should avoid to define new builtins when possible.' for stmt in stmts: if not isinstance(stmt, astroid.Import) and not isinstance(stmt, astroid.From): continue - + for imports in stmt.names: + if imports[0] == "*": + # in case of wildcard import pick the name from inside of imported module + name2 = name + else: + # pick explicitly imported name + name2 = imports[0] + if name2 not in local_names: + local_names[name2] = stmt + local_names = sorted(local_names.iteritems(), key=lambda a: a[1].fromlineno) + + checked = set() + for name, stmt in local_names: + for imports in stmt.names: # 'import imported_name' or 'from something import imported_name' - imported_name = stmt.names[0][0] + real_name = imported_name = imports[0] + if imported_name == "*": + real_name = name # 'import imported_name as as_name' - as_name = stmt.names[0][1] + as_name = imports[1] + + if real_name in checked: + continue + checked.add(real_name) - if isinstance(stmt, astroid.Import) or (isinstance(stmt, astroid.From) and not stmt.modname): + if isinstance(stmt, astroid.Import) or (isinstance(stmt, astroid.From) \ + and not stmt.modname): if as_name is None: msg = "import %s" % imported_name else: diff --git a/test/functional/unused_import.py b/test/functional/unused_import.py index 08aa0ee..c4e5186 100644 --- a/test/functional/unused_import.py +++ b/test/functional/unused_import.py @@ -4,3 +4,5 @@ import xml.sax # [unused-import] import os.path as test # [unused-import] from sys import argv as test2 # [unused-import] from sys import flags # [unused-import] +# +1:[unused-import,unused-import,unused-import] +from collections import deque, OrderedDict, Counter diff --git a/test/functional/unused_import.txt b/test/functional/unused_import.txt index 53243c1..768082c 100644 --- a/test/functional/unused_import.txt +++ b/test/functional/unused_import.txt @@ -3,3 +3,6 @@ unused-import:3::Unused import xml.sax unused-import:4::Unused os.path imported as test unused-import:5::Unused argv imported from sys as test2 unused-import:6::Unused flags imported from sys +unused-import:8::Unused Counter imported from collections +unused-import:8::Unused OrderedDict imported from collections +unused-import:8::Unused deque imported from collections -- cgit v1.2.1