-
-
Notifications
You must be signed in to change notification settings - Fork 23
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
feat: add INVALID_REPOSITORY_VALUE rule #106
Conversation
According to https://docs.npmjs.com/cli/v10/configuring-npm/package-json#repository, I'm not sure a remote URL string would work? At least when you run |
Weird, it's used to work without a hitch (perhaps they enforced in it the newer version). Anyway, I'll put a |
Don't worry about the lint fail, I'm currently tweaking the changes locally and will push some fixes soon |
I've pushed some commits that:
I think it should preserved the existing checks you did. Great addition for the I'd appreciate if you can check the changes I made and if it works for you too. |
Thanks for taking your time on refactoring this PR! It looks much cleaner than I had expected, no issues from me. As for the regex, I used these cases for testing, let me know if we missed something: [email protected]:user/project.git
https://github.com/user/project.git
http://github.com/user/project.git
[email protected]:user/project.git
https://192.168.101.127/user/project.git
http://192.168.101.127/user/project.git
ssh://[email protected]:port/path/to/repo.git/
ssh://[email protected]/path/to/repo.git/
ssh://host.xz:1234/path/to/repo.git/
ssh://host.xz/path/to/repo.git/
ssh://[email protected]/path/to/repo.git/
ssh://host.xz/path/to/repo.git/
ssh://[email protected]/~user/path/to/repo.git/
ssh://host.xz/~user/path/to/repo.git/
ssh://[email protected]/~/path/to/repo.git
ssh://host.xz/~/path/to/repo.git
git://host.xz/path/to/repo.git/
git://host.xz/~user/path/to/repo.git/
http://host.xz/path/to/repo.git/
https://host.xz/path/to/repo.git/
git+https://github.com/user/repo.git
user@server:project.git
/path/to/repo.git/
path/to/repo.git/
~/path/to/repo.git
file:///path/to/repo.git/
file://~/path/to/repo.git/
host.xz:/path/to/repo.git/ Or you can play around on Debuggex (please enable multiline flag) |
Awesome. I pushed a commit that added some test for the utilities. Thanks for the debuggex link, IIUC the current regex doesn't seem to match all the valid git urls?
Maybe we should have this set of URLs to be matched by the regex only, a valid git URL that's also valid for npm. What do you think?
(Leaving out raw IP address and port as they're a bit fishy if used, but don't mind if the final regex can capture them either) EDIT: I also found this fixture: https://github.com/npm/cli/blob/4e81a6a4106e4e125b0eefda042b75cfae0a5f23/test/lib/commands/repo.js#L115 |
I also found https://github.com/npm/hosted-git-info, maybe we should simply use that 🤔 |
I pushed a commit that updates the git URL regex so that while they may be valid git URLs per spec, they would also work for npm. The new regex also expands on the valid regexes that would previously be incorrectly marked some as invalid (e.g. ssh urls). I think this is good enough for now and I'm ready to not spend anymore time researching npm quirks 😅 Let me know if you have any more thoughts on this, otherwise I'll cut a release for this tonight. |
Personally, I would prefer if we can utilize Moreover, the RegExp seems to be unable to handle these cases correctly, hence strengthen the argument of using I can work on this if you want |
Actually I forgot to comment about It's probably easier to stick with the regex since these changes don't happen often, but if there's a neat way to reuse Also, |
Well that sucks. Seems like the library is doing too much. At this point, I think it's better to resolve this and re-visit it later when someone created an issue here about the URL 😅. I swear |
👍 Let's get this in first then. I just double checked how this change affects the popular packages listed on the site, and it seems to flag a lot of them with warnings. And strangely all those that we flagged still works on npm and displays the repository field properly. For now I'll mark |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [publint](https://publint.dev) ([source](https://togithub.com/bluwy/publint/tree/HEAD/pkg)) | [`0.2.8` -> `0.2.10`](https://renovatebot.com/diffs/npm/publint/0.2.8/0.2.10) | [![age](https://developer.mend.io/api/mc/badges/age/npm/publint/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/publint/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/publint/0.2.8/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/publint/0.2.8/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>bluwy/publint (publint)</summary> ### [`v0.2.10`](https://togithub.com/bluwy/publint/releases/tag/v0.2.10) [Compare Source](https://togithub.com/bluwy/publint/compare/v0.2.9...v0.2.10) ##### Features - Adds a new rule that validates the `"repository"` field ([https://github.com/bluwy/publint/pull/106](https://togithub.com/bluwy/publint/pull/106)) - If `"repository"` is a string, it must be one of the supported shorthand strings from the docs. - If `"repository"` is an object with `"type": "git"`, the `"url"` must be a valid [git URL](https://git-scm.com/docs/git-clone#\_git_urls) and can be [parsed by npm](https://togithub.com/npm/hosted-git-info). - The `git://` protocol for GitHub repos should not be used due [security concerns](https://github.blog/security/application-security/improving-git-protocol-security-github/). - GitHub or GitLab links should be prefixed with `git+` and postfixed with `.git`. (This is also warned by npm when publishing a package). #### New Contributors - [@​Namchee](https://togithub.com/Namchee) made their first contribution in [https://github.com/bluwy/publint/pull/106](https://togithub.com/bluwy/publint/pull/106) **Full Changelog**: bluwy/publint@v0.2.9...v0.2.10 ### [`v0.2.9`](https://togithub.com/bluwy/publint/releases/tag/v0.2.9) [Compare Source](https://togithub.com/bluwy/publint/compare/v0.2.8...v0.2.9) ##### Bug fixes - Update message when no type field is present by [@​benmccann](https://togithub.com/benmccann) ([https://github.com/bluwy/publint/pull/104](https://togithub.com/bluwy/publint/pull/104)) ##### New Contributors - [@​benmccann](https://togithub.com/benmccann) made their first contribution in [https://github.com/bluwy/publint/pull/104](https://togithub.com/bluwy/publint/pull/104) **Full Changelog**: bluwy/publint@v0.2.8...v0.2.9 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 4am on Monday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/netlify/functions). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MzguMCIsInVwZGF0ZWRJblZlciI6IjM4LjI2LjEiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbImRlcGVuZGVuY2llcyIsImphdmFzY3JpcHQiXX0=--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [publint](https://publint.dev) ([source](https://togithub.com/bluwy/publint/tree/HEAD/pkg)) | [`0.2.9` -> `0.2.10`](https://renovatebot.com/diffs/npm/publint/0.2.9/0.2.10) | [![age](https://developer.mend.io/api/mc/badges/age/npm/publint/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/publint/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/publint/0.2.9/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/publint/0.2.9/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>bluwy/publint (publint)</summary> ### [`v0.2.10`](https://togithub.com/bluwy/publint/releases/tag/v0.2.10) [Compare Source](https://togithub.com/bluwy/publint/compare/v0.2.9...v0.2.10) ##### Features - Adds a new rule that validates the `"repository"` field ([https://github.com/bluwy/publint/pull/106](https://togithub.com/bluwy/publint/pull/106)) - If `"repository"` is a string, it must be one of the supported shorthand strings from the docs. - If `"repository"` is an object with `"type": "git"`, the `"url"` must be a valid [git URL](https://git-scm.com/docs/git-clone#\_git_urls) and can be [parsed by npm](https://togithub.com/npm/hosted-git-info). - The `git://` protocol for GitHub repos should not be used due [security concerns](https://github.blog/security/application-security/improving-git-protocol-security-github/). - GitHub or GitLab links should be prefixed with `git+` and postfixed with `.git`. (This is also warned by npm when publishing a package). #### New Contributors - [@​Namchee](https://togithub.com/Namchee) made their first contribution in [https://github.com/bluwy/publint/pull/106](https://togithub.com/bluwy/publint/pull/106) **Full Changelog**: bluwy/publint@v0.2.9...v0.2.10 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/redwoodjs/redwood). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yNi4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [publint](https://publint.dev) ([source](https://togithub.com/bluwy/publint/tree/HEAD/pkg)) | [`0.2.9` -> `0.2.10`](https://renovatebot.com/diffs/npm/publint/0.2.9/0.2.10) | [![age](https://developer.mend.io/api/mc/badges/age/npm/publint/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/publint/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/publint/0.2.9/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/publint/0.2.9/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>bluwy/publint (publint)</summary> ### [`v0.2.10`](https://togithub.com/bluwy/publint/releases/tag/v0.2.10) [Compare Source](https://togithub.com/bluwy/publint/compare/v0.2.9...v0.2.10) ##### Features - Adds a new rule that validates the `"repository"` field ([https://github.com/bluwy/publint/pull/106](https://togithub.com/bluwy/publint/pull/106)) - If `"repository"` is a string, it must be one of the supported shorthand strings from the docs. - If `"repository"` is an object with `"type": "git"`, the `"url"` must be a valid [git URL](https://git-scm.com/docs/git-clone#\_git_urls) and can be [parsed by npm](https://togithub.com/npm/hosted-git-info). - The `git://` protocol for GitHub repos should not be used due [security concerns](https://github.blog/security/application-security/improving-git-protocol-security-github/). - GitHub or GitLab links should be prefixed with `git+` and postfixed with `.git`. (This is also warned by npm when publishing a package). #### New Contributors - [@​Namchee](https://togithub.com/Namchee) made their first contribution in [https://github.com/bluwy/publint/pull/106](https://togithub.com/bluwy/publint/pull/106) **Full Changelog**: bluwy/publint@v0.2.9...v0.2.10 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/redwoodjs/redwood). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yNi4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [publint](https://publint.dev) ([source](https://togithub.com/bluwy/publint/tree/HEAD/pkg)) | [`^0.2.9` -> `^0.2.10`](https://renovatebot.com/diffs/npm/publint/0.2.9/0.2.10) | [![age](https://developer.mend.io/api/mc/badges/age/npm/publint/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/publint/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/publint/0.2.9/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/publint/0.2.9/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>bluwy/publint (publint)</summary> ### [`v0.2.10`](https://togithub.com/bluwy/publint/releases/tag/v0.2.10) [Compare Source](https://togithub.com/bluwy/publint/compare/v0.2.9...v0.2.10) ##### Features - Adds a new rule that validates the `"repository"` field ([https://github.com/bluwy/publint/pull/106](https://togithub.com/bluwy/publint/pull/106)) - If `"repository"` is a string, it must be one of the supported shorthand strings from the docs. - If `"repository"` is an object with `"type": "git"`, the `"url"` must be a valid [git URL](https://git-scm.com/docs/git-clone#\_git_urls) and can be [parsed by npm](https://togithub.com/npm/hosted-git-info). - The `git://` protocol for GitHub repos should not be used due [security concerns](https://github.blog/security/application-security/improving-git-protocol-security-github/). - GitHub or GitLab links should be prefixed with `git+` and postfixed with `.git`. (This is also warned by npm when publishing a package). #### New Contributors - [@​Namchee](https://togithub.com/Namchee) made their first contribution in [https://github.com/bluwy/publint/pull/106](https://togithub.com/bluwy/publint/pull/106) **Full Changelog**: bluwy/publint@v0.2.9...v0.2.10 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/dubzzz/fast-check). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yNi4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@types/node](https://togithub.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node) ([source](https://togithub.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node)) | [`20.14.15` -> `20.16.0`](https://renovatebot.com/diffs/npm/@types%2fnode/20.14.15/20.16.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@types%2fnode/20.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@types%2fnode/20.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@types%2fnode/20.14.15/20.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@types%2fnode/20.14.15/20.16.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [lint-staged](https://togithub.com/lint-staged/lint-staged) | [`15.2.8` -> `15.2.9`](https://renovatebot.com/diffs/npm/lint-staged/15.2.8/15.2.9) | [![age](https://developer.mend.io/api/mc/badges/age/npm/lint-staged/15.2.9?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/lint-staged/15.2.9?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/lint-staged/15.2.8/15.2.9?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/lint-staged/15.2.8/15.2.9?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [publint](https://publint.dev) ([source](https://togithub.com/bluwy/publint/tree/HEAD/pkg)) | [`0.2.9` -> `0.2.10`](https://renovatebot.com/diffs/npm/publint/0.2.9/0.2.10) | [![age](https://developer.mend.io/api/mc/badges/age/npm/publint/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/publint/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/publint/0.2.9/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/publint/0.2.9/0.2.10?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>lint-staged/lint-staged (lint-staged)</summary> ### [`v15.2.9`](https://togithub.com/lint-staged/lint-staged/blob/HEAD/CHANGELOG.md#1529) [Compare Source](https://togithub.com/lint-staged/lint-staged/compare/v15.2.8...v15.2.9) ##### Patch Changes - [#​1463](https://togithub.com/lint-staged/lint-staged/pull/1463) [`b69ce2d`](https://togithub.com/lint-staged/lint-staged/commit/b69ce2ddfd5a7ae576f4fef4afc60b8a81f3c945) Thanks [@​iiroj](https://togithub.com/iiroj)! - Set the maximum number of event listeners to the number of tasks. This should silence the console warning `MaxListenersExceededWarning: Possible EventEmitter memory leak detected`. </details> <details> <summary>bluwy/publint (publint)</summary> ### [`v0.2.10`](https://togithub.com/bluwy/publint/releases/tag/v0.2.10) [Compare Source](https://togithub.com/bluwy/publint/compare/v0.2.9...v0.2.10) ##### Features - Adds a new rule that validates the `"repository"` field ([https://github.com/bluwy/publint/pull/106](https://togithub.com/bluwy/publint/pull/106)) - If `"repository"` is a string, it must be one of the supported shorthand strings from the docs. - If `"repository"` is an object with `"type": "git"`, the `"url"` must be a valid [git URL](https://git-scm.com/docs/git-clone#\_git_urls) and can be [parsed by npm](https://togithub.com/npm/hosted-git-info). - The `git://` protocol for GitHub repos should not be used due [security concerns](https://github.blog/security/application-security/improving-git-protocol-security-github/). - GitHub or GitLab links should be prefixed with `git+` and postfixed with `.git`. (This is also warned by npm when publishing a package). #### New Contributors - [@​Namchee](https://togithub.com/Namchee) made their first contribution in [https://github.com/bluwy/publint/pull/106](https://togithub.com/bluwy/publint/pull/106) **Full Changelog**: bluwy/publint@v0.2.9...v0.2.10 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - "before 4am on Monday" (UTC). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/tnez/starter-npm-pkg). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yMC4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Overview
Closes #97
This pull request adds new rule
INVALID_REPOSITORY_VALUE
that checks the validity ofrepository
field inpackage.json
.How It Works
According to the specification,
repository
may be astring
that represents a remote URL or anobject
with the following schema:This rule will check whether the repository field is defined or not. If it's defined, the rule will perform regex matching if the value is a string or check both the
url
anddirectory
property if it's an object.Caveats
type
is not checked at the moment since the specification for that property is scarce.