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_idioms.py generates bad code #47813

Closed
hawking mannequin opened this issue Aug 16, 2008 · 7 comments
Closed

fix_idioms.py generates bad code #47813

hawking mannequin opened this issue Aug 16, 2008 · 7 comments
Assignees

Comments

@hawking
Copy link
Mannequin

hawking mannequin commented Aug 16, 2008

BPO 3563
Nosy @terryjreedy, @benjaminp, @mitsuhiko
Files
  • indentation_test.diff
  • fix_idioms.patch: Patch that should fix this bug and test that this bug has been fixed.
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/benjaminp'
    closed_at = <Date 2009-10-07.21:26:21.009>
    created_at = <Date 2008-08-16.07:03:07.059>
    labels = ['expert-2to3']
    title = 'fix_idioms.py generates bad code'
    updated_at = <Date 2009-10-07.21:26:21.007>
    user = 'https://bugs.python.org/hawking'

    bugs.python.org fields:

    activity = <Date 2009-10-07.21:26:21.007>
    actor = 'benjamin.peterson'
    assignee = 'benjamin.peterson'
    closed = True
    closed_date = <Date 2009-10-07.21:26:21.009>
    closer = 'benjamin.peterson'
    components = ['2to3 (2.x to 3.x conversion tool)']
    creation = <Date 2008-08-16.07:03:07.059>
    creator = 'hawking'
    dependencies = []
    files = ['11124', '15062']
    hgrepos = []
    issue_num = 3563
    keywords = ['patch']
    message_count = 7.0
    messages = ['71199', '71217', '71777', '77298', '93672', '93673', '93725']
    nosy_count = 6.0
    nosy_names = ['collinwinter', 'terry.reedy', 'hawking', 'benjamin.peterson', 'aronacher', 'joe.amenta']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue3563'
    versions = []

    @hawking
    Copy link
    Mannequin Author

    hawking mannequin commented Aug 16, 2008

    fix_idioms.py generates bad code for conversions in try/except blocks.
    Example:
    s=(1, 2, 3)
    try:
    t = list(s)
    t.sort()
    except TypeError:
    pass

    fix_idioms.py generates this diff:
    --- test.py (original)
    +++ test.py (refactored)
    @@ -7,8 +7,7 @@
     
     s=(1, 2, 3)
     try:
    -    t = list(s)
    -    t.sort()
    -except TypeError:
    +    t = sorted(s)
    +    except TypeError:
         pass
     
    except TypeError is indented wrongly.

    @hawking hawking mannequin assigned collinwinter Aug 16, 2008
    @hawking hawking mannequin added the topic-2to3 label Aug 16, 2008
    @benjaminp
    Copy link
    Contributor

    It's due to these lines in fix_idioms:

    if next_stmt:
         next_stmt[0].set_prefix(sort_stmt.get_prefix())

    I'm not sure what the correct way to deal with this is. Anyway I'm
    attaching a test case.

    @terryjreedy
    Copy link
    Member

    Why is the statement, whatever it is, even being touched?
    Would not the same problem arise with any following outdented line?

    IOW, why not delete that pair of lines from fix_idioms.py?
    Does that break anything else in test_fixers?

    @mitsuhiko
    Copy link
    Member

    I would drop the prefix in that case or attach it to the sorted() call.

    So from this code:

        x = foo()
        # perform sorting
        x.sort()

    to

        # perform sorting
        x = sorted(foo())

    Makes more sense than sticking it after the sorted() call like it
    happens currently. This should also fix the problem with outdented
    statements such as except/finally.

    @joeamenta
    Copy link
    Mannequin

    joeamenta mannequin commented Oct 7, 2009

    Attached a patch that implements more thoroughly what appears to be the
    intended behavior.

    @joeamenta
    Copy link
    Mannequin

    joeamenta mannequin commented Oct 7, 2009

    Missed a paren in the last one... re-uploading it.

    @benjaminp benjaminp assigned benjaminp and unassigned collinwinter Oct 7, 2009
    @benjaminp
    Copy link
    Contributor

    Thanks very much for the patch; Committed in r75278.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants