Skip to content

Conversation

@chrisopperwall-qz
Copy link
Contributor

@chrisopperwall-qz chrisopperwall-qz commented Dec 22, 2025

📌 What Does This PR Do?

This PR adds a no_assign_in_argument rule to lint similar use cases to no_assign_in_condition, where an assignment expression within a function call argument is not allowed.

🔍 Context & Motivation

This rule would be helpful to warn or error in cases where it's clearer to assign a variable in its own statement, and then use that variable in the function call, instead of nesting the assignment expression in the function call argument.

🛠️ Summary of Changes

  • Feature: Added correctness/no_assign_in_argument rule.

📂 Affected Areas

  • Linter
  • Formatter
  • CLI
  • Composer Plugin
  • Dependencies
  • Documentation
  • Other (please specify):

🔗 Related Issues or PRs

📝 Notes for Reviewers

The inspiration for this change is this hhast-lint rule which discourages assignment expressions in function call arguments because

  • users may assume that new variable isn't available in the current scope/context.
  • it looks like keyword args, which exist now via a different syntax in PHP 8.

https://github.com/hhvm/hhast/blob/v3.27.4/src/Linters/NoBasicAssignmentFunctionParameterLinter.php

I'm a little new to rust, so I mostly followed the example of correctness/no_assign_in_condition, so if there's any feedback I'm happy to make changes!

@chrisopperwall-qz chrisopperwall-qz force-pushed the add-no-assignment-in-function-parameters branch from f8ba47b to b39bb3a Compare December 22, 2025 23:08
@chrisopperwall-qz chrisopperwall-qz changed the title Add rule for no assignment in function parameters feat(linter): add rule for no assignment in function parameters Dec 22, 2025
Copy link
Member

@azjezz azjezz left a comment

Choose a reason for hiding this comment

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

Hi @chrisopperwall-qz !

Thank you for the PR!

Just few notes :)

@azjezz
Copy link
Member

azjezz commented Dec 23, 2025

You can use just fix to fix clippy and/or formatting that are causing build failure now

@azjezz azjezz assigned azjezz and chrisopperwall-qz and unassigned azjezz Dec 23, 2025
@azjezz azjezz added Subject: Linter An issue or PR related to the linter. Priority: Medium This issue may be useful and needs attention. Type: Enhancement Request for additions or changes that improve existing functionality. Status: Revision Needed Multiple reviewers found issues in the PR that need addressing. labels Dec 23, 2025
@chrisopperwall-qz
Copy link
Contributor Author

Thanks, @azjezz! I'll address those comments soon

@azjezz azjezz removed the Status: Revision Needed Multiple reviewers found issues in the PR that need addressing. label Dec 23, 2025
@azjezz azjezz merged commit d1c3d32 into carthage-software:main Dec 23, 2025
@azjezz
Copy link
Member

azjezz commented Dec 23, 2025

Thank you @chrisopperwall-qz !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: Medium This issue may be useful and needs attention. Subject: Linter An issue or PR related to the linter. Type: Enhancement Request for additions or changes that improve existing functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants