Possible error when trying to fix with ICN001

[tpgillam]

I obtain errors with ICN001 when trying to apply autofix (in 'unsafe' mode) that should ideally convert the import from from A1.A2.AN import B to import A1.A2.AN.

For example, consider the following simple source file moo.py:

from math import sin

sin(42)

And then the following configuration for ruff in pyproject.toml

[project]
name = "moo"
version = "0.1.0"
readme = "README.md"
requires-python = ">=3.13"
dependencies = [
    "ruff>=0.7.4",
]

[tool.ruff.lint]
select = ["ICN001"]

[tool.ruff.lint.flake8-import-conventions.aliases]
"math.sin" = "math.sin"

I get the following error:

> uv run ruff check --fix --unsafe-fixes moo.py
error: Fix introduced a syntax error. Reverting all changes.

This indicates a bug in Ruff. If you could open an issue at:

    https://github.com/astral-sh/ruff/issues/new?title=%5BFix%20error%5D

...quoting the contents of `moo.py`, the rule codes ICN001, along with the `pyproject.toml` settings and executed command, we'd be very appreciative!

moo.py:1:18: ICN001 `math.sin` should be imported as `math.sin`
  |
1 | from math import sin
  |                  ^^^ ICN001
2 |
3 | sin(42)
  |
  = help: Alias `math.sin` to `math.sin`

Found 1 error.
[*] 1 fixable with the --fix option.

I'm not 100% sure that I'm not abusing the intention of this rule, since here I'm trying to use the rule to enforce the absence of an alias, rather than a conventional alias :)

[dylwil3]

Interesting... I think this is not quite what ICN001 was designed for. It's not possible to import (and then alias) a function like import mymodule.myfunction, which is what your user configuration is asking ICN001 to do.

Were you maybe looking for something like ICN003?

[dylwil3]

I'm also not super sold on whether to count this as a bug, since the user has required Ruff to insert the syntax error. It would be like writing a required import in the required imports linter setting that had a syntax error in it.

This is especially difficult to detect in advance because one has to resolve the module you're trying to import in order to tell whether you're attempting to alias a submodule or a function.

[dscorbett]

This should count as a bug because something of the form from x import y as x.y is a syntax error regardless of whether x.y is a submodule or a function, so it can be detected in advance. Here is an example with a submodule:

$ echo 'from collections import abc' | ruff check --config 'lint.flake8-import-conventions.aliases = {"collections.abc" = "collections.abc"}' --select ICN001 - --unsafe-fixes --fix --isolated

error: Fix introduced a syntax error. Reverting all changes.

This indicates a bug in Ruff. If you could open an issue at:

    https://github.com/astral-sh/ruff/issues/new?title=%5BFix%20error%5D

...quoting the contents of `-`, the rule codes ICN001, along with the `pyproject.toml` settings and executed command, we'd be very appreciative!

from collections import abc
-:1:25: ICN001 `collections.abc` should be imported as `collections.abc`
  |
1 | from collections import abc
  |                         ^^^ ICN001
  |
  = help: Alias `collections.abc` to `collections.abc`

Found 1 error.
[*] 1 fixable with the --fix option.

Compare I002: if the user configures it to insert a syntax error, Ruff reports it as a user error instead of a generic “This indicates a bug in Ruff” error:

$ echo 1 | ruff check --config 'lint.isort.required-imports = ["from math import sin as math.sin"]' --select I002 - --fix --isolated
error: invalid value 'lint.isort.required-imports = ["from math import sin as math.sin"]' for '--config <CONFIG_OPTION>'

  tip: A `--config` flag must either be a path to a `.toml` configuration file
       or a TOML `<KEY> = <VALUE>` pair overriding a specific configuration
       option

Could not parse the supplied argument as a `ruff.toml` configuration option:

Expected ',', found '.' at byte range 28..29
in `lint`

For more information, try '--help'.

[dylwil3]

Ah thanks for the investigation! I was mistakenly assuming that we were trying to do import a.b as ... and that's where the trouble originated.

We should certainly correct this!

As for detecting syntax errors like import math.sin: what I was trying to say is that while Ruff can detect the syntax error at the point of configuration in the example you give because it is detectable at the level of the grammar, I don't think it can detect import math.sin since it would have to know that sin was a function and not a submodule.

[tpgillam]

Just a little additional context about our objective:

We have internal modules such that there exists a function a.b.c.moo_func. And there is also d.moo_func. So we want to enforce our convention that, at all sites, we always write:

import a.b.c
import d

a.b.c.moo_func(42)
d.moo_func(42)

But there might exist a.b.c.Something that we are happy to let people import as Something due to less ambiguity, so ICN003 would be a little too restrictive in this case.

We use the ICN001 rule to enforce this -- and it does seem (perhaps coincidentally!) to do a good job of detecting what we want. It's just that the autofix doesn't work :)

[dylwil3]

The good news is that I can resolve the panic by surfacing an error to the user upon loading the configuration.

The bad news is that this breaks @tpgillam 's "hack" 😞 . Unfortunately I think I do have to prefer fixing the panic over preserving the hack, because the latter is truly not how the rule was intended to work.

If ICN003 does not help, perhaps you can open another issue describing a lint rule or configuration option that would make sense for your use case and we can consider it there? Apologies.

[tpgillam]

No apologies required! Correctness is clearly preferable, I agree.