Skip to content

feat: support TS syntax in no-unused-expressions #19564

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
Merged
merged 18 commits into from
Apr 9, 2025
Merged

feat: support TS syntax in no-unused-expressions #19564

merged 18 commits into from
Apr 9, 2025

Conversation

SwetaTanwar
Copy link
Contributor

@SwetaTanwar SwetaTanwar commented Mar 23, 2025 •
edited
Loading

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Related to #19173
Made current eslint/no-unused-expressions typescript syntax aware as per @typescript-eslint/no-unused-expressions

Is there anything you'd like reviewers to focus on?

@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Mar 23, 2025
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Mar 23, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@eslint-github-bot eslint-github-bot bot added the feature This change adds a new feature to ESLint label Mar 23, 2025
@SwetaTanwar SwetaTanwar marked this pull request as ready for review March 23, 2025 07:31
@SwetaTanwar SwetaTanwar requested a review from a team as a code owner March 23, 2025 07:31
@github-actions github-actions bot added the rule Relates to ESLint's core rules label Mar 23, 2025
@netlify Netlify
Copy link

netlify bot commented Mar 23, 2025 •
edited
Loading

✅ Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit fb6e4d1
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/67f604fbd01a8900086fd31a

@SwetaTanwar SwetaTanwar mentioned this pull request Mar 23, 2025
Open
19 tasks
@github-project-automation github-project-automation bot moved this from Needs Triage to Complete in Triage Mar 23, 2025
@SwetaTanwar SwetaTanwar reopened this Mar 23, 2025
@github-project-automation github-project-automation bot moved this from Complete to Evaluating in Triage Mar 23, 2025
@SwetaTanwar SwetaTanwar force-pushed the ts/no-unused-expressions branch from 4a3e168 to 2f2210c Compare March 23, 2025 08:24
@github-project-automation github-project-automation bot moved this from Evaluating to Complete in Triage Mar 23, 2025
@SwetaTanwar SwetaTanwar reopened this Mar 23, 2025
@github-project-automation github-project-automation bot moved this from Complete to Evaluating in Triage Mar 23, 2025
@SwetaTanwar SwetaTanwar force-pushed the ts/no-unused-expressions branch from 2f2210c to a050c53 Compare March 23, 2025 08:29
@JoshuaKGoldberg JoshuaKGoldberg changed the title feat: make no-unused-expression ts syntax aware feat: make no-unused-expressions ts syntax aware Mar 24, 2025
JoshuaKGoldberg
JoshuaKGoldberg requested changes Mar 24, 2025
View reviewed changes
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

A great start! 🚀

I found a few areas of code that can be trimmed down or outright removed without causing any unit test failures. The unit tests cover pretty much what typescript-eslint's current tests do, so my hunch is it's all dead code. But, were there other cases you wanted to cover with the extra code?

@JoshuaKGoldberg JoshuaKGoldberg moved this from Evaluating to Feedback Needed in Triage Mar 24, 2025
@JoshuaKGoldberg JoshuaKGoldberg moved this from Feedback Needed to Implementing in Triage Mar 24, 2025
@snitin315 snitin315 added accepted There is consensus among the team that this change meets the criteria for inclusion contributor pool labels Mar 24, 2025
@SwetaTanwar SwetaTanwar force-pushed the ts/no-unused-expressions branch from 2abed4a to 532a759 Compare March 24, 2025 17:55
JoshuaKGoldberg
JoshuaKGoldberg requested changes Mar 25, 2025
View reviewed changes
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

A little bit more to remove, but I think this is getting there 🙂

snitin315
snitin315 reviewed Mar 26, 2025
View reviewed changes
snitin315
snitin315 reviewed Mar 26, 2025
View reviewed changes
SwetaTanwar
SwetaTanwar commented Mar 29, 2025
View reviewed changes
mdjermanovic
mdjermanovic reviewed Mar 29, 2025
View reviewed changes
snitin315
snitin315 previously approved these changes Mar 31, 2025
View reviewed changes
Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for your contribution ❤️

Leaving it open for @mdjermanovic

JoshuaKGoldberg
JoshuaKGoldberg requested changes Mar 31, 2025
View reviewed changes
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

SO much code deletion, awesome! What a great simplification. 👏

Just one last thing from me, I think, for clarifying docs a bit.

@SwetaTanwar @JoshuaKGoldberg
chore: update docs/src/rules/no-unused-expressions.md
5ffe978 
Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
snitin315
snitin315 previously approved these changes Mar 31, 2025
View reviewed changes
mdjermanovic
mdjermanovic reviewed Apr 1, 2025
View reviewed changes
@SwetaTanwar @mdjermanovic
docs: update docs/src/rules/no-unused-expressions.md
ac3d291 
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
@mdjermanovic mdjermanovic changed the title feat: make no-unused-expressions ts syntax aware feat: support TS syntax in no-unused-expressions Apr 1, 2025
mdjermanovic
mdjermanovic reviewed Apr 1, 2025
View reviewed changes
mdjermanovic
mdjermanovic approved these changes Apr 9, 2025
View reviewed changes
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Leaving open for @JoshuaKGoldberg and @snitin315 to verify.

snitin315
snitin315 approved these changes Apr 9, 2025
View reviewed changes
Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

// @JoshuaKGoldberg

JoshuaKGoldberg
JoshuaKGoldberg approved these changes Apr 9, 2025
View reviewed changes
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

:shipit:

@snitin315 snitin315 merged commit 77d6d5b into eslint:main Apr 9, 2025
32 checks passed
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage Apr 9, 2025
@sethamus sethamus mentioned this pull request Apr 19, 2025
Merged
1 task
@snitin315 snitin315 mentioned this pull request Apr 19, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg approved these changes

@mdjermanovic mdjermanovic mdjermanovic approved these changes

@snitin315 snitin315 snitin315 approved these changes

Assignees
No one assigned
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion contributor pool feature This change adds a new feature to ESLint rule Relates to ESLint's core rules
Projects
Triage
Status: Complete
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SwetaTanwar @JoshuaKGoldberg @mdjermanovic @snitin315