Skip to content

Commit 1015d06

Browse files
Support block-level wrong-import-position pragma suppression
Signed-off-by: Emmanuel Ferdman <[email protected]>
1 parent f957d35 commit 1015d06

File tree

6 files changed

+61
-52
lines changed

6 files changed

+61
-52
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
Scope ``wrong-import-position`` pragma to specific line only.
1+
Allow ``wrong-import-position`` pragma on non-import lines to suppress following imports until the next non-import statement.
22

33
Closes #10589

pylint/checkers/imports.py

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,7 @@ def __init__(self, linter: PyLinter) -> None:
447447
self.import_graph: defaultdict[str, set[str]] = defaultdict(set)
448448
self._imports_stack: list[tuple[ImportNode, str]] = []
449449
self._first_non_import_node = None
450+
self._non_import_nodes: list = []
450451
self._module_pkg: dict[Any, Any] = (
451452
{}
452453
) # mapping of modules to the pkg they belong in
@@ -608,6 +609,7 @@ def leave_module(self, node: nodes.Module) -> None:
608609

609610
self._imports_stack = []
610611
self._first_non_import_node = None
612+
self._non_import_nodes = []
611613

612614
def compute_first_non_import_node(
613615
self,
@@ -621,12 +623,7 @@ def compute_first_non_import_node(
621623
| nodes.Try
622624
),
623625
) -> None:
624-
# if the node does not contain an import instruction, and if it is the
625-
# first node of the module, keep a track of it (all the import positions
626-
# of the module will be compared to the position of this first
627-
# instruction)
628-
if self._first_non_import_node:
629-
return
626+
# Track non-import nodes at module level to check import positions
630627
if not isinstance(node.parent, nodes.Module):
631628
return
632629
if isinstance(node, nodes.Try) and any(
@@ -644,7 +641,11 @@ def compute_first_non_import_node(
644641
]
645642
if all(valid_targets):
646643
return
647-
self._first_non_import_node = node
644+
645+
if not self._first_non_import_node:
646+
self._first_non_import_node = node
647+
648+
self._non_import_nodes.append(node)
648649

649650
visit_try = visit_assignattr = visit_assign = visit_ifexp = visit_comprehension = (
650651
visit_expr
@@ -653,12 +654,7 @@ def compute_first_non_import_node(
653654
def visit_functiondef(
654655
self, node: nodes.FunctionDef | nodes.While | nodes.For | nodes.ClassDef
655656
) -> None:
656-
# If it is the first non import instruction of the module, record it.
657-
if self._first_non_import_node:
658-
return
659-
660-
# Check if the node belongs to an `If` or a `Try` block. If they
661-
# contain imports, skip recording this node.
657+
# Record non-import instruction unless inside an If/Try block that contains imports
662658
if not isinstance(node.parent.scope(), nodes.Module):
663659
return
664660

@@ -670,7 +666,10 @@ def visit_functiondef(
670666
if any(root.nodes_of_class((nodes.Import, nodes.ImportFrom))):
671667
return
672668

673-
self._first_non_import_node = node
669+
if not self._first_non_import_node:
670+
self._first_non_import_node = node
671+
672+
self._non_import_nodes.append(node)
674673

675674
visit_classdef = visit_for = visit_while = visit_functiondef
676675

@@ -699,17 +698,37 @@ def _check_position(self, node: ImportNode) -> None:
699698
700699
Send a message if `node` comes before another instruction
701700
"""
702-
# if a first non-import instruction has already been encountered,
703-
# it means the import comes after it and therefore is not well placed
701+
# Check if import comes after a non-import statement
704702
if self._first_non_import_node:
705-
if self.linter.is_message_enabled("wrong-import-position", node.fromlineno):
706-
self.add_message(
707-
"wrong-import-position", node=node, args=node.as_string()
708-
)
709-
else:
703+
# Check for inline pragma on the import line
704+
if not self.linter.is_message_enabled("wrong-import-position", node.fromlineno):
710705
self.linter.add_ignored_message(
711706
"wrong-import-position", node.fromlineno, node
712707
)
708+
return
709+
710+
# Check for pragma on the preceding non-import statement
711+
most_recent_non_import = None
712+
for non_import_node in self._non_import_nodes:
713+
if non_import_node.fromlineno < node.fromlineno:
714+
most_recent_non_import = non_import_node
715+
else:
716+
break
717+
718+
if most_recent_non_import:
719+
check_line = most_recent_non_import.fromlineno
720+
if not self.linter.is_message_enabled("wrong-import-position", check_line):
721+
self.linter.add_ignored_message(
722+
"wrong-import-position", check_line, most_recent_non_import
723+
)
724+
self.linter.add_ignored_message(
725+
"wrong-import-position", node.fromlineno, node
726+
)
727+
return
728+
729+
self.add_message(
730+
"wrong-import-position", node=node, args=node.as_string()
731+
)
713732

714733
def _record_import(
715734
self,
Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
"""Checks that disabling 'wrong-import-position' only affects the specific line.
2-
3-
A pragma on a non-import statement should not affect subsequent import statements.
4-
This demonstrates the correct behavior after fixing the bug.
5-
"""
1+
"""Test wrong-import-position pragma on non-import statement."""
62
# pylint: disable=unused-import
73

8-
CONSTANT = True # pylint: disable=wrong-import-position
4+
import os
5+
import sys
6+
7+
CONSTANT_A = False # pylint: disable=wrong-import-position
8+
import time
99

10-
import sys # [wrong-import-position]
10+
CONSTANT_B = True
11+
import logging # [wrong-import-position]
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
wrong-import-position:10:0:10:10::"Import ""import sys"" should be placed at the top of the module":UNDEFINED
1+
wrong-import-position:11:0:11:14::"Import ""import logging"" should be placed at the top of the module":UNDEFINED
Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,16 @@
1-
"""Test that wrong-import-position pragma suppression is correctly scoped."""
2-
# pylint: disable=unused-import,invalid-name
1+
"""Test wrong-import-position pragma scoping."""
2+
# pylint: disable=unused-import
33

4-
import logging
4+
import os
55
import sys
66

7-
# This pragma should not affect subsequent import statements
8-
logger = logging.getLogger() # pylint: disable=wrong-import-position
9-
logging.basicConfig(level='DEBUG')
7+
# Pragma on non-import suppresses following imports until next non-import
8+
CONSTANT_A = False # pylint: disable=wrong-import-position
9+
import time
1010

11-
logger.debug('importing modules...')
12-
import os # [wrong-import-position]
13-
import pathlib # [wrong-import-position]
14-
import random # [wrong-import-position]
15-
logger.debug('done importing')
11+
CONSTANT_B = True
12+
import logging # [wrong-import-position]
1613

17-
# Test that pragma on import line works correctly (this import should not be flagged)
18-
constant_var = "test"
14+
# Inline pragma on import line
15+
CONSTANT_C = 42
1916
import json # pylint: disable=wrong-import-position
20-
21-
# Test that subsequent imports are not affected by the pragma above
22-
import csv # [wrong-import-position]
23-
import re # [wrong-import-position]
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
wrong-import-position:12:0:12:9::"Import ""import os"" should be placed at the top of the module":UNDEFINED
2-
wrong-import-position:13:0:13:14::"Import ""import pathlib"" should be placed at the top of the module":UNDEFINED
3-
wrong-import-position:14:0:14:13::"Import ""import random"" should be placed at the top of the module":UNDEFINED
4-
wrong-import-position:22:0:22:10::"Import ""import csv"" should be placed at the top of the module":UNDEFINED
5-
wrong-import-position:23:0:23:9::"Import ""import re"" should be placed at the top of the module":UNDEFINED
1+
wrong-import-position:12:0:12:14::"Import ""import logging"" should be placed at the top of the module":UNDEFINED

0 commit comments

Comments
 (0)