SwiftLint icon indicating copy to clipboard operation
SwiftLint copied to clipboard

`unused-parameter`: Support configuring parameter names to ignore

Open sindresorhus opened this issue 1 year ago • 7 comments

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.

sindresorhus avatar Aug 08 '24 20:08 sindresorhus

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.

SimplyDanny avatar Aug 09 '24 08:08 SimplyDanny

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 avatar Aug 09 '24 18:08 sindresorhus

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.

sindresorhus avatar Aug 09 '24 18:08 sindresorhus

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?

SimplyDanny avatar Aug 09 '24 20:08 SimplyDanny

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 avatar Aug 09 '24 21:08 sindresorhus

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.

sindresorhus avatar Aug 09 '24 21:08 sindresorhus

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 avatar Aug 10 '24 10:08 SimplyDanny