feat: add the "valid-license" rule

[xenobytezero]

PR Checklist

• Addresses an existing open issue: fixes 🚀 Feature: Bring in a valid-license rule #830
• That issue was marked as status: accepting prs
• Steps in CONTRIBUTING.md were taken

Overview

Adds a new rule that enforces the 'licence' value in the package.json to be a specific value, or one of a selection of values.

Proposing this as a new rule to eliminate my usage of npm-package-json-lint, since this was the only feature I needed from it.

I did have this as a custom rule in my own ESLint plugin, but I found out it's very difficult to apply multiple plugins to the same file and I am already using this plugin.

This rule could expand in scope to allow for any value to be validated in the same way.

[JoshuaKGoldberg]

Thanks for sending this! This was a good prod to close out the conversation in #51 about how we want to approach all these requirements. I posted #51 (comment).

Left a couple of initial review comments here in case you feel motivated to work on this before we're certain this is the direction for the package. Cheers!

[JoshuaKGoldberg]

Lots of unexpected changes in this file - could you please revert them?

https://www.joshuakgoldberg.com/blog/split-out-unrelated-changes

[xenobytezero]

Looks like there was some confusion around tabs vs spaces between Prettier and editorconfig (they seem to specify different settings) so all the tabs in plugin.ts got nuked. Fixed.

[michaelfaith]

It looks like the rule and rule test files aren't named consistently with the rule itself requires-license vs require-license

[xenobytezero]

It looks like the rule and rule test files aren't named consistently with the rule itself requires-license vs require-license

Rookie error, fixed.

[michaelfaith]

Thanks for your patience as we worked out the long-term strategy for the rules in this plugin. What we landed on in #51 is that we want to have separate rules for requiring a property and for validating the correctness of that property (if it exists). That way people can use one or the other, or both.

So for require-licence, the rule should just purely test for the existence of the property. And a separate rule valid-licence would check that if the property exists that it is valid. The current state of this PR has both requiredness and validation checks. So, in order for this to land, it would need to be reduced to just the check for requiredness. Does that make sense?

[michaelfaith]

Actually, on second though, why don't we convert this one into the valid-license rule, since most of the logic is oriented towards that? The main difference now, though, is that there's no check for required-ness, since the other rule will handle that. Let me know if any of that is unclear and I'll help clear things up.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.78%. Comparing base (4c72153) to head (47ccb5a).
Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #786      +/-   ##
==========================================
+ Coverage   98.71%   98.78%   +0.07%     
==========================================
  Files          19       20       +1     
  Lines        1163     1234      +71     
  Branches      139      146       +7     
==========================================
+ Hits         1148     1219      +71     
  Misses         15       15              

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

[xenobytezero]

Actually, on second though, why don't we convert this one into the valid-license rule, since most of the logic is oriented towards that? The main difference now, though, is that there's no check for required-ness, since the other rule will handle that. Let me know if any of that is unclear and I'll help clear things up.

No problem, can rework this rule to check for validity.

• Obviously checking validity would require existence of the property in order to validate the value, and from what I can gather in reading Bring in a "require-*"/"no-*" kind of rule #51 the rule should also fail for non-existance?
• Should it fail in the way it does now?
• Should I split out the require logic into it's own thing since most of the requires rules will be very similar?

[michaelfaith]

and from what I can gather in reading Bring in a "require-"/"no-" kind of rule #51 the rule should also fail for non-existance?

Actually that went back and forth a couple of times (my bad 😅). Where we landed is that valid- rules will not fail if the property isn't present. That will be the responsibility of the require- rules. valid- rules will just validate that, if the property is present, its value is valid. It won't do anything if there isn't a property with that name.

[michaelfaith]

• Should I split out the require logic into it's own thing since most of the requires rules will be very similar?

I already put up a PR that lays the groundwork for our approach to require- flavor rules: #851 Definitely welcome your input!

[xenobytezero]

Just working on this now, looks like the changes would be essentially cosmetic to match what was discussed.

The rule currently has a check that if the property is present that it also must be a string. I don't think there is anything in the current discussion about where type checking of the value should live in require- or valid-?

[michaelfaith]

The require- rules won't do any checking of the value. It'll just simply check whether the property is presentor not.