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

unused-parameter: Support configuring parameter names to ignore #5741

Open
2 tasks done
sindresorhus opened this issue Aug 8, 2024 · 7 comments
Open
2 tasks done
Labels
enhancement Ideas for improvements of existing features and rules. good first issue Issue to be taken up by new contributors yet unfamiliar with the project.

Comments

@sindresorhus
Copy link

New Issue Checklist

Feature or Enhancement Proposal

The rule has a large number of violations where you either cannot change or remove the parameters. The most common case are delegate methods. It would be useful to be able to specify a list of parameter names to ignore. That would silence a lot of the most common cases.

Personally, I would add coder, sender, and context.

@SimplyDanny
Copy link
Collaborator

You can always replace the parameter with _ which is what the rule suggests as an alternative. A list of names feels like a too broad way of excluding cases.

@sindresorhus
Copy link
Author

The problem with replacing the parameter name with _ is that it will often be unclear what it was for, and if I ever need it again, I will have to look it up.

How about allowing prefixing the parameter with _ to ignore it? That's how it works with TypeScript. Then it's clear it's unused, while still being readable and easy to use again when needed.

@sindresorhus
Copy link
Author

Here's an example of where I would almost never use context but I cannot change it (other than adding a internal _ label, but that's ugly):

struct FooView: NSViewRepresentable {
	func makeNSView(context: Context) -> NSViewType {
		// ...
	}

	func updateNSView(_ nsView: NSViewType, context: Context) {}
}

I pretty much want to ignore the context for all of these. Maybe it could also allow specifying the type name, to be more specific, or maybe a selector.

@SimplyDanny
Copy link
Collaborator

The problem with replacing the parameter name with _ is that it will often be unclear what it was for, and if I ever need it again, I will have to look it up.

If the function satisfies a protocol requirement or overrides a method from a super class, the parameter can anyway not be omitted. Thus, its name is still there. It cannot be replaced with just _, but its internal name can be _. All other functions should not have unused parameters. That's the whole purpose of the rule. 😉

How about allowing prefixing the parameter with _ to ignore it? That's how it works with TypeScript. Then it's clear it's unused, while still being readable and easy to use again when needed.

You say func f(x _: Int) {} is ugly and func f(x _x: Int) {} is better? I don't really see this, but adding an option to consider variables starting with an underscore as admissibly unused should be doable.

Here's an example of where I would almost never use context but I cannot change it (other than adding a internal _ label, but that's ugly):

struct FooView: NSViewRepresentable {
	func makeNSView(context: Context) -> NSViewType {
		// ...
	}

	func updateNSView(_ nsView: NSViewType, context: Context) {}
}

I pretty much want to ignore the context for all of these. Maybe it could also allow specifying the type name, to be more specific, or maybe a selector.

I get that. However, the rule can be run with --fix so that the many places will be automatically silenced. Wouldn't it be confusing if the rule wasn't to trigger on a special set of parameters names? Including the type is also too vague considering Self, typealias, generics, ...

What about excluding whole functions by name from the analysis given that this is typically annoying for a defined set of framework methods?

@sindresorhus
Copy link
Author

You say func f(x _: Int) {} is ugly and func f(x _x: Int) {} is better? I don't really see this, but adding an option to consider variables starting with an underscore as admissibly unused should be doable.

You generally have func f(x: Int) or func f(for x: Int) type signatures. For the former, I agree that func f(x _: Int) is the best choice, but for func f(for x: Int), it becomes unclear what it means if you convert the x to _, and func f(for _x: Int) would be better then.

@sindresorhus
Copy link
Author

What about excluding whole functions by name from the analysis given that this is typically annoying for a defined set of framework methods?

Just the function name can have the same problem with being too broad as you argued with just using the parameter name.

@SimplyDanny
Copy link
Collaborator

You say func f(x _: Int) {} is ugly and func f(x _x: Int) {} is better? I don't really see this, but adding an option to consider variables starting with an underscore as admissibly unused should be doable.

You generally have func f(x: Int) or func f(for x: Int) type signatures. For the former, I agree that func f(x _: Int) is the best choice, but for func f(for x: Int), it becomes unclear what it means if you convert the x to _, and func f(for _x: Int) would be better then.

Fair enough. An option to support this style should be fine then.

What about excluding whole functions by name from the analysis given that this is typically annoying for a defined set of framework methods?

Just the function name can have the same problem with being too broad as you argued with just using the parameter name.

Indeed. Glad you mention it. 😅 Up to now, I don't really see an elegant solution.

@SimplyDanny SimplyDanny added enhancement Ideas for improvements of existing features and rules. good first issue Issue to be taken up by new contributors yet unfamiliar with the project. labels Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ideas for improvements of existing features and rules. good first issue Issue to be taken up by new contributors yet unfamiliar with the project.
Projects
None yet
Development

No branches or pull requests

2 participants