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

Rename autofix introduces errors when name is used as a shorthand property #55356

Closed
bradzacher opened this issue Aug 14, 2023 · 5 comments
Closed
Labels
Question An issue which isn't directly actionable in code

Comments

@bradzacher
Copy link
Contributor

πŸ”Ž Search Terms

rename autofix object shorthand property

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried

⏯ Playground Link

https://www.typescriptlang.org/play?#code/FA4MwVwOwYwFwJYHsoAIxKQCgJQC5UBvVABwCckSBTMuATwIGc4yEoBzVAXyOFX9QwUzUhWq06qALyoARLIDcfAWSpwIZNIWUCB5SjXoAaHdyVdgQA

πŸ’» Code

function foo(): { property: string } {
    const property = "";
    return {
        property,
    };
}

πŸ™ Actual behavior

Screen.Recording.2023-08-14.at.13.16.38.mov

πŸ™‚ Expected behavior

When autofix renaming a type property it should consider shorthands and fix appropriately so that it can avoid introducing type errors due to incorrect naming.

  function foo(): { property: string } {
                    ^^^^^^^^ rename to property2
      const property = "";
      return {
-         property,
+         property2: property,
      };
  }
  function foo(): { property: string } {
      const property = "";
            ^^^^^^^^ rename to property2
      return {
-         property,
+         property: property2,
      };
  }

Optionally, it could be extended to be (hugely more complex) for some cases:

  function foo(): { property: string } {
                    ^^^^^^^^ rename to property2
-     const property = "";
+     const property2 = "";
      return {
-         property,
+         property2,
      };
  }
@RyanCavanaugh
Copy link
Member

It sounds like you have this option disabled
image

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Aug 15, 2023
@bradzacher
Copy link
Contributor Author

That is the default configuration - the screen recording was taken from the playground.

It seems counter-intuitive to me that the default config refactors to broken code.

@RyanCavanaugh
Copy link
Member

It might also seem counter-intuitive that renaming a local identifier should cause a runtime behavior change. But both can't be true (or false) at once.

See #29238 (comment)

@bradzacher
Copy link
Contributor Author

I'm not quite sure I understand - how is this a runtime behaviour change?

  return {
-   prop,
+   prop: prop2,
  };

The shorthand assignment is syntactic sugar isn't it? So this change has no observable runtime side-effect.

I understand that renaming the prop does have runtime side-effects - which is the intent behind the refactor. But renaming a variable and using the long-hand property assignment to maintain the existing property name doesn't, right?

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Question" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

3 participants