Skip to content

feat(language-service): support to fix missing required inputs diagno… #50911

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

feat(language-service): support to fix missing required inputs diagno… #50911

wants to merge 2 commits into from
+551 −0

Conversation

ivanwonder
Copy link
Contributor

@ivanwonder ivanwonder commented Jul 1, 2023 •
edited
Loading

…stic

Support to add the missing required inputs into the element and append after the last attribute of the element.

But it skips the structural directive shorthand attribute. For example, <a *ngFor=""></a>, its shorthand is <ng-template ngFor>, the valueSpan of the ngFor is empty, and the info is lost, so it can't use to insert the missing attribute after it.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Jul 1, 2023
@ivanwonder ivanwonder force-pushed the codefix-for-require-input branch from aa9d62a to f0c16b0 Compare July 1, 2023 04:03
@ivanwonder ivanwonder marked this pull request as ready for review July 1, 2023 04:17
@pullapprove pullapprove bot requested review from alxhub and JoostK July 1, 2023 04:17
@pkozlowski-opensource pkozlowski-opensource added the area: language-service Issues related to Angular's VS Code language service label Jul 5, 2023
@ngbot ngbot bot added this to the Backlog milestone Jul 5, 2023
@thePunderWoman thePunderWoman marked this pull request as draft May 21, 2025 15:24
@thePunderWoman
Copy link
Contributor

@ivanwonder Sorry for the long wait on getting this looked at. If you're still willing to get this landed, can you rebase and resolve the conflicts. It looks like a nice feature to get landed.

@thePunderWoman thePunderWoman force-pushed the codefix-for-require-input branch from f0c16b0 to b5f7c12 Compare June 4, 2025 15:42
@thePunderWoman
Copy link
Contributor

@ivanwonder Sorry for the long wait on getting this looked at. If you're still willing to get this landed, can you rebase and resolve the conflicts. It looks like a nice feature to get landed.

Actually, nevermind on the rebase. I was able to resolve those for you.

@thePunderWoman thePunderWoman marked this pull request as ready for review June 4, 2025 15:43
@thePunderWoman thePunderWoman requested a review from atscott June 4, 2025 15:43
@thePunderWoman thePunderWoman added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Jun 4, 2025
@thePunderWoman thePunderWoman force-pushed the codefix-for-require-input branch 2 times, most recently from de4f346 to 8360b43 Compare June 4, 2025 16:21
@thePunderWoman
Copy link
Contributor

@ivanwonder Yeah, looks like even with resolving the conflicts, this is broken. Can you take a look?

@thePunderWoman thePunderWoman added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 4, 2025
@ivanwonder
Copy link
Contributor Author

@ivanwonder Yeah, looks like even with resolving the conflicts, this is broken. Can you take a look?

OK, I'll take a look.

@ivanwonder
Copy link
Contributor Author

@ivanwonder Yeah, looks like even with resolving the conflicts, this is broken. Can you take a look?

@thePunderWoman Done. It seems that the NgModule does not recognize the standalone component in the test case.

@thePunderWoman thePunderWoman removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jun 5, 2025
@ivanwonder ivanwonder force-pushed the codefix-for-require-input branch 2 times, most recently from 06b6bd1 to efc2a14 Compare June 5, 2025 14:05
@atscott atscott added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jun 11, 2025
atscott
atscott approved these changes Jun 11, 2025
View reviewed changes
@angular-robot angular-robot bot requested a review from atscott June 12, 2025 02:30
@ivanwonder ivanwonder force-pushed the codefix-for-require-input branch from 9200747 to 67763fb Compare June 12, 2025 02:42
atscott
atscott requested changes Jun 16, 2025
View reviewed changes
@ivanwonder ivanwonder force-pushed the codefix-for-require-input branch from 67763fb to d6b93fc Compare June 17, 2025 08:03
@ivanwonder
feat(language-service): support to fix missing required inputs diagno…
618020b 
…stic

Support to add the missing required inputs into the element and append
after the last attribute of the element.

But it skips the structural directive shorthand attribute. For example,
`<a *ngFor=""></a>`, its shorthand is `<ng-template ngFor>`, the `valueSpan`
of the `ngFor` is empty, and the info is lost, so it can't use to insert
the missing attribute after it.
@ivanwonder ivanwonder force-pushed the codefix-for-require-input branch from d6b93fc to 618020b Compare June 17, 2025 08:16
ivanwonder
ivanwonder commented Jun 17, 2025
View reviewed changes
packages/language-service/src/codefixes/fix_missing_required_inputs.ts Outdated
Comment on lines 293 to 304
/**
* If the input is a decorator, return the type of the symbol; if there is a
* transform for the input, return the transform type.
*
* If the input is a signal, the real input type is wrapped by the signal type.
*/
function getInputType(
symbol: ts.Symbol,
typeCheck: ts.TypeChecker,
isSignal: boolean,
transformTypeNode: ts.TypeNode | undefined,
): ts.Type {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to support the input transform and the signal type.

@atscott This needs to be reviewed again.

@ivanwonder ivanwonder requested a review from atscott June 17, 2025 08:21
atscott
atscott approved these changes Jun 17, 2025
View reviewed changes
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Jun 18, 2025
@crisbeto
Copy link
Member

This PR was merged into the repository by commit 5d2e859.

The changes were merged into the following branches: main

@crisbeto crisbeto closed this in 5d2e859 Jun 19, 2025
@ivanwonder ivanwonder deleted the codefix-for-require-input branch June 19, 2025 10:07
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@atscott atscott atscott approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: language-service Issues related to Angular's VS Code language service detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.
5 participants
@ivanwonder @thePunderWoman @crisbeto @atscott @pkozlowski-opensource