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

relative-url-style: Improve fix and check logic #1748

Merged
merged 8 commits into from
Mar 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/rules/relative-url-style.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<!-- RULE_NOTICE -->
✅ *This rule is part of the [recommended](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config) config.*

🔧 *This rule is [auto-fixable](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems).*
🔧💡 *This rule is [auto-fixable](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) and provides [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).*
<!-- /RULE_NOTICE -->

When using a relative URL in [`new URL()`](https://developer.mozilla.org/en-US/docs/Web/API/URL/URL), the URL should either never or always use the `./` prefix consistently.
Expand Down
2 changes: 1 addition & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ Each rule has emojis denoting:
| [prefer-top-level-await](docs/rules/prefer-top-level-await.md) | Prefer top-level await over top-level promises and async function calls. | | | 💡 |
| [prefer-type-error](docs/rules/prefer-type-error.md) | Enforce throwing `TypeError` in type checking conditions. | ✅ | 🔧 | |
| [prevent-abbreviations](docs/rules/prevent-abbreviations.md) | Prevent abbreviations. | ✅ | 🔧 | |
| [relative-url-style](docs/rules/relative-url-style.md) | Enforce consistent relative URL style. | ✅ | 🔧 | |
| [relative-url-style](docs/rules/relative-url-style.md) | Enforce consistent relative URL style. | ✅ | 🔧 | 💡 |
| [require-array-join-separator](docs/rules/require-array-join-separator.md) | Enforce using the separator argument with `Array#join()`. | ✅ | 🔧 | |
| [require-number-to-fixed-digits-argument](docs/rules/require-number-to-fixed-digits-argument.md) | Enforce using the digits argument with `Number#toFixed()`. | ✅ | 🔧 | |
| [require-post-message-target-origin](docs/rules/require-post-message-target-origin.md) | Enforce using the `targetOrigin` argument with `window.postMessage()`. | | | 💡 |
Expand Down
126 changes: 92 additions & 34 deletions rules/relative-url-style.js
Original file line number Diff line number Diff line change
@@ -1,79 +1,134 @@
'use strict';
const {getStaticValue} = require('eslint-utils');
const {newExpressionSelector} = require('./selectors/index.js');
const {replaceStringLiteral} = require('./fix/index.js');

const MESSAGE_ID_NEVER = 'never';
const MESSAGE_ID_ALWAYS = 'always';
const MESSAGE_ID_REMOVE = 'remove';
const messages = {
[MESSAGE_ID_NEVER]: 'Remove the `./` prefix from the relative URL.',
[MESSAGE_ID_ALWAYS]: 'Add a `./` prefix to the relative URL.',
[MESSAGE_ID_REMOVE]: 'Remove leading `./`.',
};

const selector = [
const templateLiteralSelector = [
newExpressionSelector({name: 'URL', argumentsLength: 2}),
' > .arguments:first-child',
' > TemplateLiteral.arguments:first-child',
].join('');
const literalSelector = [
newExpressionSelector({name: 'URL', argumentsLength: 2}),
' > Literal.arguments:first-child',
].join('');

const DOT_SLASH = './';
const TEST_URL_BASE = 'https://example.com/';
const isSafeToAddDotSlash = url => {
const TEST_URL_BASES = [
'https://example.com/a/b/',
'https://example.com/a/b.html',
];
const isSafeToAddDotSlashToUrl = (url, base) => {
try {
return new URL(url, TEST_URL_BASE).href === new URL(`${DOT_SLASH}${url}`, TEST_URL_BASE).href;
return new URL(url, base).href === new URL(DOT_SLASH + url, base).href;
} catch {}

return false;
};

function removeDotSlash(node) {
const isSafeToAddDotSlash = (url, bases = TEST_URL_BASES) => bases.every(base => isSafeToAddDotSlashToUrl(url, base));
const isSafeToRemoveDotSlash = (url, bases = TEST_URL_BASES) => bases.every(base => isSafeToAddDotSlashToUrl(url.slice(DOT_SLASH.length), base));

function canAddDotSlash(node, context) {
const url = node.value;
if (url.startsWith(DOT_SLASH) || url.startsWith('.') || url.startsWith('/')) {
return false;
}

const baseNode = node.parent.arguments[1];
const staticValueResult = getStaticValue(baseNode, context.getScope());

if (
node.type === 'TemplateLiteral'
&& node.quasis[0].value.raw.startsWith(DOT_SLASH)
staticValueResult
&& typeof staticValueResult.value === 'string'
&& isSafeToAddDotSlash(url, [staticValueResult.value])
) {
const firstPart = node.quasis[0];
return fixer => {
const start = firstPart.range[0] + 1;
return fixer.removeRange([start, start + 2]);
};
return true;
}

if (node.type !== 'Literal' || typeof node.value !== 'string') {
return;
}
return isSafeToAddDotSlash(url);
}

if (!node.raw.slice(1, -1).startsWith(DOT_SLASH)) {
return;
function canRemoveDotSlash(node, context) {
const rawValue = node.raw.slice(1, -1);
if (!rawValue.startsWith(DOT_SLASH)) {
return false;
}

return fixer => replaceStringLiteral(fixer, node, '', 0, 2);
}
const baseNode = node.parent.arguments[1];
const staticValueResult = getStaticValue(baseNode, context.getScope());

function addDotSlash(node) {
if (node.type !== 'Literal' || typeof node.value !== 'string') {
return;
if (
staticValueResult
&& typeof staticValueResult.value === 'string'
&& isSafeToRemoveDotSlash(node.value, [staticValueResult.value])
) {
return true;
}

const url = node.value;
return isSafeToRemoveDotSlash(node.value);
}

if (url.startsWith(DOT_SLASH)) {
function addDotSlash(node, context) {
if (!canAddDotSlash(node, context)) {
return;
}

if (
url.startsWith('.')
|| url.startsWith('/')
|| !isSafeToAddDotSlash(url)
) {
return fixer => replaceStringLiteral(fixer, node, DOT_SLASH, 0, 0);
}

function removeDotSlash(node, context) {
if (!canRemoveDotSlash(node, context)) {
return;
}

return fixer => replaceStringLiteral(fixer, node, DOT_SLASH, 0, 0);
return fixer => replaceStringLiteral(fixer, node, '', 0, 2);
}

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
const style = context.options[0] || 'never';
return {[selector](node) {
const fix = (style === 'never' ? removeDotSlash : addDotSlash)(node);

const listeners = {};

// TemplateLiteral are not always safe to remove `./`, but if it's starts with `./` we'll report
if (style === 'never') {
listeners[templateLiteralSelector] = function (node) {
const firstPart = node.quasis[0];
if (!firstPart.value.raw.startsWith(DOT_SLASH)) {
return;
}

return {
node,
messageId: style,
suggest: [
{
messageId: MESSAGE_ID_REMOVE,
fix(fixer) {
const start = firstPart.range[0] + 1;
return fixer.removeRange([start, start + 2]);
},
},
],
};
};
}

listeners[literalSelector] = function (node) {
if (typeof node.value !== 'string') {
return;
}

const fix = (style === 'never' ? removeDotSlash : addDotSlash)(node, context);

if (!fix) {
return;
Expand All @@ -84,7 +139,9 @@ const create = context => {
messageId: style,
fix,
};
}};
};

return listeners;
};

const schema = [
Expand All @@ -103,6 +160,7 @@ module.exports = {
description: 'Enforce consistent relative URL style.',
},
fixable: 'code',
hasSuggestions: true,
schema,
messages,
},
Expand Down
11 changes: 10 additions & 1 deletion test/relative-url-style.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ test.snapshot({
'new URL("./foo", base, extra)',
'new URL("./foo", ...[base])',
'new NOT_URL("./foo", base)',
'new NOT_URL("./", base)',
'new URL("./", base)',
'new URL("./", "https://example.com/a/b/c.html")',
'const base = new URL("./", import.meta.url)',
'new URL',
'new URL(0, base)',
// Not checking this case
'new globalThis.URL("./foo", base)',
'const foo = "./foo"; new URL(foo, base)',
Expand All @@ -29,9 +34,9 @@ test.snapshot({
invalid: [
'new URL("./foo", base)',
'new URL(\'./foo\', base)',
'new URL("./", base)',
'new URL("././a", base)',
'new URL(`./${foo}`, base)',
'new URL("./", "https://example.com/a/b/")',
],
});

Expand All @@ -45,7 +50,10 @@ test.snapshot({
'new URL("foo", base, extra)',
'new URL("foo", ...[base])',
'new NOT_URL("foo", base)',
'new URL("", base)',
'new URL("", "https://example.com/a/b.html")',
'/* 2 */ new URL',
'new URL(0, base2)',
// Not checking this case
'new globalThis.URL("foo", base)',
'new URL(`${foo}`, base2)',
Expand All @@ -66,5 +74,6 @@ test.snapshot({
invalid: [
'new URL("foo", base)',
'new URL(\'foo\', base)',
'new URL("", "https://example.com/a/b/")',
].map(code => ({code, options: alwaysAddDotSlashOptions})),
});
56 changes: 39 additions & 17 deletions test/snapshots/relative-url-style.mjs.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,51 +37,49 @@ Generated by [AVA](https://avajs.dev).
`

## Invalid #3
1 | new URL("./", base)
1 | new URL("././a", base)

> Output

`␊
1 | new URL("", base)␊
1 | new URL("a", base)␊
`

> Error 1/1

`␊
> 1 | new URL("./", base)␊
| ^^^^ Remove the \`./\` prefix from the relative URL.␊
> 1 | new URL("././a", base)␊
| ^^^^^^^ Remove the \`./\` prefix from the relative URL.␊
`

## Invalid #4
1 | new URL("././a", base)

> Output

`␊
1 | new URL("a", base)␊
`
1 | new URL(`./${foo}`, base)

> Error 1/1

`␊
> 1 | new URL("././a", base)␊
| ^^^^^^^ Remove the \`./\` prefix from the relative URL.␊
> 1 | new URL(\`./${foo}\`, base)␊
| ^^^^^^^^^^ Remove the \`./\` prefix from the relative URL.␊
--------------------------------------------------------------------------------␊
Suggestion 1/1: Remove leading \`./\`.␊
1 | new URL(\`${foo}\`, base)␊
`

## Invalid #5
1 | new URL(`./${foo}`, base)
1 | new URL("./", "https://example.com/a/b/")

> Output

`␊
1 | new URL(\`${foo}\`, base)␊
1 | new URL("", "https://example.com/a/b/")␊
`

> Error 1/1

`␊
> 1 | new URL(\`./${foo}\`, base)␊
| ^^^^^^^^^^ Remove the \`./\` prefix from the relative URL.␊
> 1 | new URL("./", "https://example.com/a/b/")␊
| ^^^^ Remove the \`./\` prefix from the relative URL.␊
`

## Invalid #1
Expand Down Expand Up @@ -131,3 +129,27 @@ Generated by [AVA](https://avajs.dev).
> 1 | new URL('foo', base)␊
| ^^^^^ Add a \`./\` prefix to the relative URL.␊
`

## Invalid #3
1 | new URL("", "https://example.com/a/b/")

> Options

`␊
[␊
"always"␊
]␊
`

> Output

`␊
1 | new URL("./", "https://example.com/a/b/")␊
`

> Error 1/1

`␊
> 1 | new URL("", "https://example.com/a/b/")␊
| ^^ Add a \`./\` prefix to the relative URL.␊
`
Binary file modified test/snapshots/relative-url-style.mjs.snap
Binary file not shown.