Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parse error message for number parsing #724

Merged
merged 1 commit into from
Aug 4, 2022
Merged

Conversation

zzl0
Copy link
Contributor

@zzl0 zzl0 commented Jul 16, 2022

Summary

TLDR:

  • Fixed a missing 'f' for f-string issue.
  • Tried to fix the issue by creating a codemod, but my solution has many false positives, so it is not included in this PR code changes.

When I was working on another PR (#704), I encountered a missing 'f' for f-string issue (code link). I was wondering if I can write a codemod command to solve this automatically. Below code is my first attempt to solve this issue, it can find the missing 'f' for f-string typos, but it has many false positives and I realized it's not simple problem as I thought.

This PR is to fix the found missing 'f' for f-string issue and also to see if you have tried to solve this issue before.

import re
import difflib

import libcst as cst
import libcst.matchers as m
from libcst.codemod import VisitorBasedCodemodCommand
from libcst.codemod._context import CodemodContext
from libcst.metadata.scope_provider import ScopeProvider

PATTERN = re.compile('\{([^}]+)\}')


class FixFormattedStringCommand(VisitorBasedCodemodCommand):
    DESCRIPTION: str = "Fix f-string."
    METADATA_DEPENDENCIES = (ScopeProvider,)

    def leave_SimpleString(
        self,
        original_node: cst.SimpleString,
        updated_node: cst.SimpleString
    ) -> cst.FormattedString:
        str_val = updated_node.value
        if vars := PATTERN.findall(str_val):
            scope = self.get_metadata(ScopeProvider, original_node)
            if not all(scope.get_qualified_names_for(v) for v in vars):
                return updated_node
            formatted_string = cst.parse_expression(f"f{str_val}")
            return formatted_string
        return updated_node


if __name__ == '__main__':
    py_source = """
def foo() -> str:
    a = "123"
    return "x: {type} = {val}",

def bar() -> str:
    a = "123"
    return "{a}"
"""

    source_tree = cst.parse_module(py_source)
    wrapper = cst.MetadataWrapper(source_tree)

    transformer = FixFormattedStringCommand(CodemodContext())
    modified_tree = wrapper.visit(transformer)

    print(
        "".join(
            difflib.unified_diff(py_source.splitlines(1), modified_tree.code.splitlines(1))
        )
    )

Below is a test run:

$ python libcst/codemod/commands/fix_fstring.py
---
+++
@@ -5,4 +5,4 @@

 def bar() -> str:
     a = "123"
-    return "{a}"
+    return f"{a}"

Test Plan

👀

@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 Jul 16, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #724 (fbbe91b) into main (367b14b) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main     #724   +/-   ##
=======================================
  Coverage   94.82%   94.82%           
=======================================
  Files         247      247           
  Lines       25799    25799           
=======================================
  Hits        24463    24463           
  Misses       1336     1336           
Impacted Files Coverage Δ
libcst/_parser/conversions/expression.py 98.10% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 367b14b...fbbe91b. Read the comment docs.

@zsol
Copy link
Member

zsol commented Aug 4, 2022

Thanks for fixing this! As for the codemod, I think it doesn't need to call get_qualified_names_for, it's enough to just check if v in scope. But this sounds more like a linter than a codemod

@zsol zsol changed the title Fix missing 'f' for f-strings. Fix parse error message for number parsing Aug 4, 2022
@zsol zsol merged commit 47e5ea1 into Instagram:main Aug 4, 2022
@zzl0
Copy link
Contributor Author

zzl0 commented Aug 6, 2022

Thanks @zsol for reviewing this PR. I agree it more like linter, and a PR for pylint tried to add this feature pylint-dev/pylint#4787

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants