chore: add exported-function-args-maximum Deno Style Guide linter plugin

[timreichen]

Towards #6551

[codecov]

Codecov Report

Attention: Patch coverage is 94.23077% with 6 lines in your changes missing coverage. Please review.

Project coverage is 94.75%. Comparing base (f8894da) to head (facb153).
Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
_tools/lint_plugin.ts	83.78%	6 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6561   +/-   ##
=======================================
  Coverage   94.74%   94.75%           
=======================================
  Files         583      574    -9     
  Lines       46478    46516   +38     
  Branches     6523     6530    +7     
=======================================
+ Hits        44036    44074   +38     
  Misses       2399     2399           
  Partials       43       43           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[iuioiua]

IMO, we should review all the instances where this rule is ignored. It's a bit of a shame that we ignore this rule in some parts of the codebase. It's a very reasonable rule and not adhering to it could pose a few little backwards compatibility challenges in the future.

[iuioiua]

Sidenote: bums me out a little that we don't follow this rule in @std/assert 😝

[iuioiua]

Non-blocker: we should have this function just accept an object.

[BlackAsLight]

A max of two arguments seems kind of arbitrary and I think we may find many instances where it doesn't make sense

[kt3k]

A max of two arguments seems kind of arbitrary and I think we may find many instances where it doesn't make sense

Note this rule only applies to the public APIs (though this lint rule checks more than that). Internal utilities don't need to follow this.

The rule exists from the beginning of the project, and the maintainers keep checking the compliance to it (for example, see this discussion #4651 (comment) ). I think it's reasonable to keep this rule.

[kt3k]

• The rule allows the function in last position. The error on pooledMap looks a false positive.
• I think we should exclude files starting with _. They are usually internal utils and errors on them should be false positives.
• I think we should exclude @std/bytes. The usages of number as the 3rd arg are intentional (discussed in feat(bytes): @std/[email protected] #4651 )
• @std/internal should be excluded as they are not part of public API
• I think we should also exclude functions starting with assert. They generally don't follow this rule.
• deleteCookie follows the rule, but it seems getting the error

Only the items below look like truly exceptions to this rule to me:

• reduceGroups
• runningReduce
• encodeIntoBase32
• encodeIntoBase64
• encodeVarint
• chown
• chownSync
• utime
• utimeSync
• toTransformStream
• stub
• createCapture
• resliceBufferWithPadding
• createTextureWithData

[iuioiua]

• I think we should exclude files starting with _. They are usually internal utils and errors on them should be false positives.

• @std/internal should be excluded as they are not part of public API

I don't think this rule should exclusively apply to the public API. The maintainer can benefit a little by having these symbols accept objects by having removing argument position as a pain point. The changes to do this are trivial too.

• I think we should exclude @std/bytes. The usages of number as the 3rd arg are intentional (discussed in feat(bytes): @std/[email protected] #4651 )

We still need to update the style guide.

[kt3k]

• I think we should exclude files starting with _. They are usually internal utils and errors on them should be false positives.

• @std/internal should be excluded as they are not part of public API

I don't think this rule should exclusively apply to the public API. The maintainer can benefit a little by having these symbols accept objects by having removing argument position as a pain point. The changes to do this are trivial too.

This rule is very uncommon and restrictive in my view. I guess many contributors would feel uncomfortable if this rule is enforced everywhere. (See @BlackAsLight's comment for example)

• I think we should exclude @std/bytes. The usages of number as the 3rd arg are intentional (discussed in feat(bytes): @std/[email protected] #4651 )

We still need to update the style guide.

That exception is already covered by the 2nd paragraph of the 2nd point.

An optional parameter that's not in an options object might be acceptable if there is only one, and it seems inconceivable that we would add more optional parameters in the future.

[BlackAsLight]

I don't think this rule should exclusively apply to the public API. The maintainer can benefit a little by having these symbols accept objects by having removing argument position as a pain point. The changes to do this are trivial too.

In this example here, https://github.com/denoland/std/blob/main/encoding/_common64.ts#L54-L60, I'd find it more painful to have to needlessly put it in an object than write it in a particular order.

[timreichen]

I don't think this rule should exclusively apply to the public API. The maintainer can benefit a little by having these symbols accept objects by having removing argument position as a pain point. The changes to do this are trivial too.

In this example here, https://github.com/denoland/std/blob/main/encoding/_common64.ts#L54-L60, I'd find it more painful to have to needlessly put it in an object than write it in a particular order.

To be honest I think the rule is great especially for functions like the one you pointed out. I have no idea what the arguments mean without getting into the meat of the function. JSDocs are only required for public apis, so this rule actually helps to understand the internals too.

[iuioiua]

I don't think this rule should exclusively apply to the public API. The maintainer can benefit a little by having these symbols accept objects by having removing argument position as a pain point. The changes to do this are trivial too.

In this example here, https://github.com/denoland/std/blob/main/encoding/_common64.ts#L54-L60, I'd find it more painful to have to needlessly put it in an object than write it in a particular order.

To be honest I think the rule is great especially for functions like the one you pointed out. I have no idea what the arguments mean without getting into the meat of the function. JSDocs are only required for public apis, so this rule actually helps to understand the internals too.

I agree.

[iuioiua]

LGTM, barring one nit.