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.

Sign up for GitHub

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

Jump to bottom

fix(eslint-plugin): [keyword-spacing] unexpected space before/after in import type #6095

Merged
merged 7 commits into from Dec 2, 2022
Merged

fix(eslint-plugin): [keyword-spacing] unexpected space before/after in import type #6095

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
78f3749
fix/issue6063-keyword-spacing-unexpected-space-before
Nov 26, 2022
91c18ff
remove some redundancy
Nov 26, 2022
4d58a46
Merge remote-tracking branch 'upstream/main' into fix/issue6063-keywo…
Nov 26, 2022
ac5f9bd
Add overrides for type token
Nov 27, 2022
78a634c
Merge remote-tracking branch 'upstream/main' into fix/issue6063-keywo…
Nov 27, 2022
e70260a
Merge remote-tracking branch 'upstream/main' into fix/issue6063-keywo…
Dec 1, 2022
e8d5efc
test
Dec 1, 2022
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
7 changes: 6 additions & 1 deletion packages/eslint-plugin/src/rules/keyword-spacing.ts
View file
Open in desktop
@@ -1,5 +1,5 @@
import type { TSESTree } from '@typescript-eslint/utils';
import { AST_TOKEN_TYPES } from '@typescript-eslint/utils';
import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils';

import * as util from '../util';
import { getESLintCoreRule } from '../util/getESLintCoreRule';
Expand Down Expand Up @@ -56,6 +56,11 @@ export default util.createRule<Options, MessageIds>({
): void {
const typeToken = sourceCode.getFirstToken(node, { skip: 1 })!;
const punctuatorToken = sourceCode.getTokenAfter(typeToken)!;
if (
node.specifiers?.[0]?.type === AST_NODE_TYPES.ImportDefaultSpecifier
) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought draft means you won't review it (Sorry, I guess it meant something else to me).

I opened it for review because I can't think of a way to solve it without adding an override option for the type keyword or should it use the existing option for the import keyword.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I just didn't realize there was a question associated with this! Sorry, i misinterpreted the discussion. Will think on it 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, actually, definitely yes. Since the type option doesn't exist in ESLint core and is TypeScript-specific, we should add in a feature for it. Yes please 😄

Copy link
Contributor Author

@omril1 omril1 Nov 26, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any example I can look at how to do that? The types seem looser than the schema validation (typed as overrides?: Record<string, { before?: boolean; after?: boolean;}> but adding type: { after: true } to object breaks the schema validation

OK, I found something similar to the no-magic-numbers rule

const spacesBetweenTypeAndPunctuator =
punctuatorToken.range[0] - typeToken.range[1];
if (after && spacesBetweenTypeAndPunctuator === 0) {
Expand Down
100 changes: 98 additions & 2 deletions packages/eslint-plugin/tests/rules/keyword-spacing.test.ts
View file
Open in desktop
Expand Up @@ -116,12 +116,78 @@ ruleTester.run('keyword-spacing', rule, {
},
{
code: 'import type { foo } from "foo";',
options: [{ overrides: { as: {} } }],
options: [BOTH],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: "import type * as Foo from 'foo'",
options: [{ overrides: { as: {} } }],
options: [BOTH],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: "import type{SavedQueries} from './SavedQueries.js';",
options: [
{
before: true,
after: false,
overrides: {
from: { after: true },
},
},
],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: "import type{SavedQueries} from'./SavedQueries.js';",
options: [
{
before: true,
after: false,
},
],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: "import type http from 'node:http';",
options: [
{
before: true,
after: false,
overrides: {
from: { after: true },
},
},
],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: "import type http from'node:http';",
options: [
{
before: true,
after: false,
},
],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: 'import type {} from "foo";',
options: [BOTH],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: 'import type { foo1, foo2 } from "foo";',
options: [BOTH],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: 'import type { foo1 as _foo1, foo2 as _foo2 } from "foo";',
options: [BOTH],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
{
code: 'import type { foo as bar } from "foo";',
options: [BOTH],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
},
],
Expand Down Expand Up @@ -190,5 +256,35 @@ ruleTester.run('keyword-spacing', rule, {
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [{ messageId: 'unexpectedBefore', data: { value: '*' } }],
},
{
code: "import type {SavedQueries} from './SavedQueries.js';",
output: "import type{SavedQueries} from './SavedQueries.js';",
options: [
{
before: true,
after: false,
overrides: {
from: { after: true },
},
},
],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [{ messageId: 'unexpectedBefore', data: { value: '{' } }],
},
{
code: "import type {SavedQueries} from './SavedQueries.js';",
output: "import type{SavedQueries} from'./SavedQueries.js';",
options: [
{
before: true,
after: false,
},
],
parserOptions: { ecmaVersion: 6, sourceType: 'module' },
errors: [
{ messageId: 'unexpectedBefore', data: { value: '{' } },
{ messageId: 'unexpectedAfter', data: { value: 'from' } },
],
},
],
});