Skip to content

Commit

Permalink
- Fixed #1301: Import headings in nested sections leads to check errors
Browse files Browse the repository at this point in the history
  • Loading branch information
timothycrosley committed Jul 11, 2020
1 parent 046a62f commit d744e7f
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 25 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -3,6 +3,9 @@ Changelog

NOTE: isort follows the [semver](https://semver.org/) versioning standard.

### 5.0.9 July 11, 2020
- Fixed #1301: Import headings in nested sections leads to check errors

### 5.0.8 July 11, 2020
- Fixed #1277 & #1278: New line detection issues on Windows.
- Fixed #1294: Fix bundled git hook.
Expand Down
21 changes: 8 additions & 13 deletions isort/api.py
Expand Up @@ -8,18 +8,10 @@
from warnings import warn

from . import io, output, parse
from .exceptions import (
ExistingSyntaxErrors,
FileSkipComment,
FileSkipSetting,
IntroducedSyntaxErrors,
)
from .format import (
ask_whether_to_apply_changes_to_file,
format_natural,
remove_whitespace,
show_unified_diff,
)
from .exceptions import (ExistingSyntaxErrors, FileSkipComment,
FileSkipSetting, IntroducedSyntaxErrors)
from .format import (ask_whether_to_apply_changes_to_file, format_natural,
remove_whitespace, show_unified_diff)
from .io import Empty
from .place import module as place_module # skipcq: PYL-W0611 (intended export of public API)
from .place import module_with_reason as place_module_with_reason # skipcq: PYL-W0611 (^)
Expand Down Expand Up @@ -398,7 +390,7 @@ def _sort_imports(
else:
stripped_line = line.strip()
if stripped_line and not line_separator:
line_separator = line[len(line.rstrip()) :]
line_separator = line[len(line.rstrip()) :].replace(" ", "").replace("\t", "")

for file_skip_comment in FILE_SKIP_COMMENTS:
if file_skip_comment in line:
Expand Down Expand Up @@ -451,6 +443,9 @@ def _sort_imports(
isort_off = True
elif stripped_line == "# isort: split":
not_imports = True
elif stripped_line in config.section_comments and not import_section:
import_section += line
indent = line[: -len(line.lstrip())]
elif not (stripped_line or contains_imports):
if add_imports and not indent:
import_section += line_separator.join(add_imports) + line_separator
Expand Down
6 changes: 4 additions & 2 deletions isort/hooks.py
Expand Up @@ -57,9 +57,11 @@ def git_hook(strict: bool = False, modify: bool = False) -> int:
staged_cmd = ["git", "show", f":{filename}"]
staged_contents = get_output(staged_cmd)

if not api.check_code_string(staged_contents, file_path=Path(filename)):
if not api.check_code_string(
staged_contents, file_path=Path(filename), settings_path=Path(filename).parent
):
errors += 1
if modify:
api.sort_file(filename)
api.sort_file(filename, settings_path=Path(filename).parent)

return errors if strict else 0
2 changes: 1 addition & 1 deletion isort/output.py
Expand Up @@ -185,7 +185,7 @@ def sorted_imports(
line,
in_quote="",
index=len(formatted_output),
section_comments=parsed.section_comments,
section_comments=config.section_comments,
)
if not should_skip and line.strip():
if (
Expand Down
9 changes: 3 additions & 6 deletions isort/parse.py
Expand Up @@ -80,7 +80,7 @@ def _strip_syntax(import_string: str) -> str:


def skip_line(
line: str, in_quote: str, index: int, section_comments: List[str]
line: str, in_quote: str, index: int, section_comments: Tuple[str, ...]
) -> Tuple[bool, str]:
"""Determine if a given line should be skipped.
Expand Down Expand Up @@ -134,7 +134,6 @@ class ParsedContent(NamedTuple):
original_line_count: int
line_separator: str
sections: Any
section_comments: List[str]


def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedContent:
Expand All @@ -143,7 +142,6 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte
in_lines = contents.split(line_separator)
out_lines = []
original_line_count = len(in_lines)
section_comments = [f"# {heading}" for heading in config.import_headings.values()]
if config.old_finders:
finder = FindersManager(config=config).find
else:
Expand Down Expand Up @@ -172,10 +170,10 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte
index += 1
statement_index = index
(skipping_line, in_quote) = skip_line(
line, in_quote=in_quote, index=index, section_comments=section_comments
line, in_quote=in_quote, index=index, section_comments=config.section_comments
)

if line in section_comments and not skipping_line:
if line in config.section_comments and not skipping_line:
if import_index == -1:
import_index = index - 1
continue
Expand Down Expand Up @@ -435,5 +433,4 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte
original_line_count=original_line_count,
line_separator=line_separator,
sections=config.sections,
section_comments=section_comments,
)
16 changes: 14 additions & 2 deletions isort/settings.py
Expand Up @@ -11,7 +11,8 @@
from distutils.util import strtobool as _as_bool
from functools import lru_cache
from pathlib import Path
from typing import Any, Callable, Dict, FrozenSet, Iterable, List, Optional, Pattern, Set, Tuple
from typing import (Any, Callable, Dict, FrozenSet, Iterable, List, Optional,
Pattern, Set, Tuple)
from warnings import warn

from . import stdlibs
Expand Down Expand Up @@ -206,11 +207,15 @@ def __init__(
config: Optional[_Config] = None,
**config_overrides,
):
self._known_patterns: Optional[List[Tuple[Pattern[str], str]]] = None
self._section_comments: Optional[Tuple[str, ...]] = None

if config:
config_vars = vars(config).copy()
config_vars.update(config_overrides)
config_vars["py_version"] = config_vars["py_version"].replace("py", "")
config_vars.pop("_known_patterns")
config_vars.pop("_section_comments")
super().__init__(**config_vars) # type: ignore
return

Expand Down Expand Up @@ -318,7 +323,6 @@ def __init__(
combined_config.pop(f"{IMPORT_HEADING_PREFIX}{import_heading_key}")
combined_config["import_headings"] = import_headings

self._known_patterns: Optional[List[Tuple[Pattern[str], str]]] = None
super().__init__(sources=tuple(sources), **combined_config) # type: ignore

def is_skipped(self, file_path: Path) -> bool:
Expand Down Expand Up @@ -378,6 +382,14 @@ def known_patterns(self):

return self._known_patterns

@property
def section_comments(self) -> Tuple[str, ...]:
if self._section_comments is not None:
return self._section_comments

self._section_comments = tuple(f"# {heading}" for heading in self.import_headings.values())
return self._section_comments

@staticmethod
def _parse_known_pattern(pattern: str) -> List[str]:
"""Expand pattern if identified as a directory and return found sub packages"""
Expand Down
1 change: 0 additions & 1 deletion tests/test_parse.py
Expand Up @@ -36,7 +36,6 @@ def test_file_contents():
original_line_count,
_,
_,
_,
) = parse.file_contents(TEST_CONTENTS, config=Config(default_section=""))
assert "\n".join(in_lines) == TEST_CONTENTS
assert "import" not in "\n".join(out_lines)
Expand Down
17 changes: 17 additions & 0 deletions tests/test_regressions.py
Expand Up @@ -245,3 +245,20 @@ def test_windows_newline_issue_1278():
"from ..log import CheckLoggingAdapter, init_logging\r\n\r\n init_logging()\r\n"
"except ImportError:\r\n pass\r\n"
)


def test_check_never_passes_with_indented_headings_1301():
"""Test to ensure that test can pass even when there are indented headings.
See: https://github.com/timothycrosley/isort/issues/1301
"""
assert isort.check_code(
"""
try:
# stdlib
import logging
from os import abc, path
except ImportError:
pass
""",
import_heading_stdlib="stdlib",
)

0 comments on commit d744e7f

Please sign in to comment.