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

[omril1]

PR Checklist

• Addresses an existing open issue: fixes Bug: [@typescript-eslint/keyword-spacing] Unexpected space(s) before "{" in import type #6063
• That issue was marked as accepting prs
• Steps in CONTRIBUTING.md were taken

Overview

This is very hard to define correctly as the type keyword sits between already supported tokens from the base rule that have their own defined before/after.
For now, this is how I think the defining cases should be but I'm open to changing them or add more as you see fit

• I'm not sure if I should add support for an overrides option for type, and how to do it with extending the base rule

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit e8d5efc. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 47 targets

• Node 14 - nx test @typescript-eslint/eslint-plugin --coverage=false
• Node 18 - nx test @typescript-eslint/eslint-plugin
• Node 18 - nx test @typescript-eslint/visitor-keys
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• nx run-many --target=typecheck --all --parallel
• nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx test @typescript-eslint/utils
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx test @typescript-eslint/type-utils
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• nx run-many --target=lint --all --parallel
• Node 18 - nx test @typescript-eslint/eslint-plugin-tslint
• Node 18 - nx test @typescript-eslint/typescript-estree
• Node 14 - nx test @typescript-eslint/ast-spec --coverage=false
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx test @typescript-eslint/eslint-plugin-internal
• Node 18 - nx test @typescript-eslint/parser
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/parser --coverage=false
• Node 14 - nx test @typescript-eslint/type-utils --coverage=false
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/visitor-keys --coverage=false
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/eslint-plugin-tslint --coverage=false
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/typescript-estree --coverage=false
• Node 18 - nx test @typescript-eslint/scope-manager
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/utils --coverage=false
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx test @typescript-eslint/ast-spec
• nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 18 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx test @typescript-eslint/scope-manager --coverage=false
• Node 14 - nx test @typescript-eslint/eslint-plugin-internal --coverage=false
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14 - nx run-many --target=build --all --parallel --exclude website --exclude website
• nx run-many --target=build --all --parallel --exclude website --exclude website

---

Sent with 💌 from NxCloud.

[typescript-eslint]

Thanks for the PR, @omril1!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

---

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

[netlify]

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	e8d5efc
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/6388fa61a7e44c000986d84d
😎 Deploy Preview	https://deploy-preview-6095--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[codecov]

Codecov Report

Merging #6095 (e8d5efc) into main (1dba257) will decrease coverage by 0.00%.
The diff coverage is 91.66%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6095      +/-   ##
==========================================
- Coverage   91.27%   91.26%   -0.01%     
==========================================
  Files         366      366              
  Lines       12419    12427       +8     
  Branches     3632     3638       +6     
==========================================
+ Hits        11335    11342       +7     
  Misses        774      774              
- Partials      310      311       +1     

Flag	Coverage Δ
unittest	91.26% <91.66%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ackages/eslint-plugin/src/rules/keyword-spacing.ts	96.77% <91.66%> (-3.23%)	⬇️

[JoshuaKGoldberg]

Did you mean to put this as a draft? Since it doesn't yet address the original issue, I'll convert it. Let me know if anything's unclear. And thanks for working on this!

[JoshuaKGoldberg]

I'm confused. The preview deploy's playground still shows an unexpected space for one of the three issue's sample cases: https://deploy-preview-6095--typescript-eslint.netlify.app/play/#ts=4.9.3&sourceType=module&code=FASwtgDg9gTgLgAjgTwgUwQbwQZQIYBuaAJgIoCuaMIaAzggL4IBmMUYCARAHQD0+RMpWp1uAK1qcA3KEixEKdAgAWcOBABMLNh04A7KMTQAuVeo3TZ0eElQYzEbey4GjptREtA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Aa0UIHcB7aACa1kxAIZlmAc3RgA2uGw5uo6E0gAaBYqyLskAEaIAZv0Qz8cRJt17RR-Imgyjo+MitbdkXgDdHVARQZHRscRDczDBDQnDsHJwwLWERPGwBfaxioaER8WFVg1NDIOMdzSyLFDMq9C0JCrNt7MsSKxurGyDJRfDIACwbGqFKEsBcImogOrK7Rd0HOkfLkybBpmK7eJmR8BZml1pX2zI38Puhebj2Ng7AklOPVyCRdqNXY5tH71fXigV5rsVbt9Hp1eEZATYSp9lg8sr8oXNIV5gW14SdikZmK54PU3kNhjDDnCYgivEYLgBbZGKaHxWE-DFQyiU4j8V6Yd6E+nExlPRAADzZ0A50RuRLuaNJTK8gRcCFFXLpLUlR3hNTSRU1igAugpNWkgA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA. Am I missing something?

import type { SavedQueries } from "./SavedQueries.js";

[omril1]

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.

[JoshuaKGoldberg]

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

[JoshuaKGoldberg]

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 😄

[omril1]

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

[bradzacher]

thanks so much for following up to fix this!