Skip to content

Commit

Permalink
relative-url-style: Improve fix and check logic (#1748)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fisker
fisker committed Mar 17, 2022
1 parent 9bfde90 commit f406795
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 54 deletions.
2 changes: 1 addition & 1 deletion docs/rules/relative-url-style.md
View file
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
View file
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
View file
@@ -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
View file
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
View file
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
View file
Binary file not shown.

0 comments on commit f406795

Please sign in to comment.