pylance-release icon indicating copy to clipboard operation
pylance-release copied to clipboard

"Cannot find a symbol to move" when moving symbol from/to filename with leading digit

Open debonte opened this issue 1 year ago • 1 comments

Environment data

  • Language Server version: 2024.3.100

Code Snippet

1.py

CONSTANT = 1

Repro Steps

  1. In a new directory, create 1.py as shown above.
  2. Create empty file 2.py
  3. In 1.py, right-click on CONSTANT > Refactor... > Move symbol to ...
  4. Select 2.py as the destination.

Expected behavior

Either CONSTANT is moved to 2.py or the user is provided with an explanation of why it can't be moved.

Actual behavior

image

...which obviously doesn't help the user.

The underlying issue is that "1" and "2" are not valid module names. Digits are allowed in identifier names, but not as the first character. See https://docs.python.org/3.11/reference/lexical_analysis.html#identifiers.

For move to succeed, both the source and target filenames must be legal module names. However, I believe that's technically only required if the move results in src->dest or dest->src imports being added. In this simple scenario, CONSTANT = 1 could have been successfully moved because no imports are needed.

I found this extremely confusing and only figured out why the move wasn't allowed by debugging Pylance. So while the simplest fix would probably be to not show the Move Symbol refactorings in this scenario, that would be equally confusing IMO. This is another argument for addressing https://github.com/microsoft/pylance-release/issues/5551 in my mind.

debonte avatar Mar 14 '24 01:03 debonte

In this simple scenario, CONSTANT = 1 could have been successfully moved because no imports are needed.

problem is this is expensive to figure out. so not something we would do in code action provider since it will require find all references

so, I think practical mitigation might be showing better message? either in message box after execution or in code action menu

heejaechang avatar Mar 14 '24 07:03 heejaechang