Instagram / LibCST

A concrete syntax tree parser and serializer library for Python that preserves many aspects of Python's abstract syntax tree
https://libcst.readthedocs.io/
Other
1.53k stars 188 forks source link

Clear warnings for each file in codemod cli #1184

Closed kiri11 closed 1 month ago

kiri11 commented 2 months ago

Summary

Original idea belongs to @jschavesr. I am just resubmitting his abandoned PR. Fixes the issue: https://github.com/Instagram/LibCST/issues/1183

Context warning messages were not being cleaned between files, so warnings from one files could be passed to other files in the context. Because of that warnings were being shown wrongly in the summary and the total number of warnings was wrong as well.

I also encountered another error in Windows CI. Seems like codemodding small files happens faster than the timer counts more than zero (probably less precision on Windows). So fixing that, too.

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "D:\a\LibCST\LibCST\libcst\tool.py", line 689, in <module>
    main(os.environ.get("LIBCST_TOOL_COMMAND_NAME", "libcst.tool"), sys.argv[1:])
  File "D:\a\LibCST\LibCST\libcst\tool.py", line 684, in main
    return lookup.get(args.action or None, _invalid_command)(proc_name, command_args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\a\LibCST\LibCST\libcst\tool.py", line 420, in _codemod_impl
    result = parallel_exec_transform_with_prettyprint(
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\a\LibCST\LibCST\libcst\codemod\_cli.py", line 659, in parallel_exec_transform_with_prettyprint
    progress.print(successes + failures + skips)
  File "D:\a\LibCST\LibCST\libcst\codemod\_cli.py", line 398, in print
    f"{self.ERASE_CURRENT_LINE}{self._human_seconds(elapsed_time)} {percent:.{self.pretty_precision}f}% complete, {self.estimate_completion(elapsed_time, finished, left)} estimated for {left} files to go...",
                                                                                                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\a\LibCST\LibCST\libcst\codemod\_cli.py", line 430, in estimate_completion
    fps = files_finished / elapsed_seconds
          ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
ZeroDivisionError: float division by zero

Test Plan

Before:

Running convert_format_to_fstring.ConvertFormatStringCommand on three files with identical contents:

def baz() -> str:
    return "{}: {}".format(*baz)

All three are the same, so they should get one warning each. But instead every following file gets more warnings copied from previous files:

LibCST> python -m libcst.tool codemod convert_format_to_fstring.ConvertFormatStringCommand tmp
Calculating full-repo metadata...
Executing codemod...
Codemodding tmp\mod1.py
WARNING: Unsupported field_name 0 in format() call
Successfully codemodded tmp\mod1.py with warnings

Codemodding tmp\mod2.py
WARNING: Unsupported field_name 0 in format() call
WARNING: Unsupported field_name 0 in format() call
Successfully codemodded tmp\mod2.py with warnings

Codemodding tmp\mod3.py
WARNING: Unsupported field_name 0 in format() call
WARNING: Unsupported field_name 0 in format() call
WARNING: Unsupported field_name 0 in format() call
Successfully codemodded tmp\mod3.py with warnings

Finished codemodding 3 files!
 - Transformed 3 files successfully.
 - Skipped 0 files.
 - Failed to codemod 0 files.
 - 6 warnings were generated.

The new test fails:

FAIL: test_warning_messages_several_files (libcst.codemod.tests.test_codemod_cli.TestCodemodCLI.test_warning_messages_several_files)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "libcst\codemod\tests\test_codemod_cli.py", line 92, in test_warning_messages_several_files
    self.assertIn(
AssertionError: '- 3 warnings were generated.' not found in "Calculating full-repo metadata... ... \n - 6 warnings were generated.\n"

After:

Each file generates one warning correctly:

LibCST> python -m libcst.tool codemod convert_format_to_fstring.ConvertFormatStringCommand tmp
Calculating full-repo metadata...
Executing codemod...
Codemodding tmp\mod1.py
WARNING: Unsupported field_name 0 in format() call
Successfully codemodded tmp\mod1.py with warnings

Codemodding tmp\mod2.py
WARNING: Unsupported field_name 0 in format() call
Successfully codemodded tmp\mod2.py with warnings

Codemodding tmp\mod3.py
WARNING: Unsupported field_name 0 in format() call
Successfully codemodded tmp\mod3.py with warnings

Finished codemodding 3 files!
 - Transformed 3 files successfully.
 - Skipped 0 files.
 - Failed to codemod 0 files.
 - 3 warnings were generated.

And the new test passes.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.

Project coverage is 91.26%. Comparing base (56cd1f9) to head (71986bf).

Files Patch % Lines
libcst/codemod/_cli.py 57.14% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1184 +/- ## ======================================= Coverage 91.26% 91.26% ======================================= Files 261 261 Lines 26883 26898 +15 ======================================= + Hits 24535 24549 +14 - Misses 2348 2349 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

zsol commented 1 month ago

Thanks!