Skip to content

allow configuring empty formatter lists in codemod CLI#1319

Merged
zsol merged 2 commits intoInstagram:mainfrom
ngoldbaum:empty-formatter
Apr 2, 2025
Merged

allow configuring empty formatter lists in codemod CLI#1319
zsol merged 2 commits intoInstagram:mainfrom
ngoldbaum:empty-formatter

Conversation

@ngoldbaum
Copy link
Contributor

@ngoldbaum ngoldbaum commented Apr 1, 2025

Summary

Right now if you have an empty formatter list, you'll crash on this line:

goldbaum at Nathans-MBP in ~/Documents/numpy on format-to-fstring
± python3 -m libcst.tool codemod --no-format convert_format_to_fstring.ConvertFormatStringCommand .
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/Users/goldbaum/.pyenv/versions/3.13.2/lib/python3.13/site-packages/libcst/tool.py", line 688, in <module>
    main(os.environ.get("LIBCST_TOOL_COMMAND_NAME", "libcst.tool"), sys.argv[1:])
    ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/goldbaum/.pyenv/versions/3.13.2/lib/python3.13/site-packages/libcst/tool.py", line 683, in main
    return lookup.get(args.action or None, _invalid_command)(proc_name, command_args)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/goldbaum/.pyenv/versions/3.13.2/lib/python3.13/site-packages/libcst/tool.py", line 380, in _codemod_impl
    if os.path.basename(config["formatter"][0]) in ("black", "black.exe"):
                        ~~~~~~~~~~~~~~~~~~~^^^
IndexError: list index out of range

I think having an empty formatter list is a reasonable thing. You'll still error out later unless you don't pass --no-format to the CLI tool.

Test Plan

Right now the codemod CLI doesn't have great tests so it's not easy to add a new test. Happy to talk about that though since I think having better end-to-end tests will make writing multithreaded stress tests easier.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 1, 2025
@ngoldbaum
Copy link
Contributor Author

ngoldbaum commented Apr 1, 2025

Right now the codemod CLI doesn't have great tests

I guess I missed test_codemod_cli.py earlier, although I don't think there are any tests for the config file yet.

@zsol zsol merged commit a4804cf into Instagram:main Apr 2, 2025
31 checks passed
@zsol
Copy link
Contributor

zsol commented Apr 2, 2025

Thanks!

@ngoldbaum ngoldbaum mentioned this pull request Apr 2, 2025
Merged
ngoldbaum added a commit to ngoldbaum/LibCST that referenced this pull request Apr 8, 2025
@ngoldbaum
allow configuring empty formatter lists in codemod CLI (Instagram#1319)
c57437c 
* allow configuring empty formatter lists

* appease linter
jorenham pushed a commit to jorenham/LibCST that referenced this pull request Feb 3, 2026
@ngoldbaum @jorenham
allow configuring empty formatter lists in codemod CLI (Instagram#1319)
dcb4d62 
* allow configuring empty formatter lists

* appease linter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ngoldbaum @zsol @facebook-github-bot