Skip to content

feat: add svelte/@typescript-eslint/no-unnecessary-condition rule #262

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

Merged
merged 7 commits into from
Sep 26, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/real-wasps-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-svelte": minor
---

feat: add `svelte/@typescript-eslint/no-unnecessary-condition` rule
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
/tests/fixtures/rules/valid-compile/valid/babel
/tests/fixtures/rules/valid-compile/valid/ts
/tests/fixtures/rules/prefer-style-directive
/tests/fixtures/rules/@typescript-eslint
/.svelte-kit
/svelte.config-dist.js
/build
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -322,10 +322,11 @@ These rules relate to style guidelines, and are therefore quite subjective:

## Extension Rules

These rules extend the rules provided by ESLint itself to work well in Svelte:
These rules extend the rules provided by ESLint itself, or other plugins to work well in Svelte:

| Rule ID | Description | |
|:--------|:------------|:---|
| [svelte/@typescript-eslint/no-unnecessary-condition](https://ota-meshi.github.io/eslint-plugin-svelte/rules/@typescript-eslint/no-unnecessary-condition/) | disallow conditionals where the type is always truthy or always falsy | :wrench: |
| [svelte/no-inner-declarations](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-inner-declarations/) | disallow variable or `function` declarations in nested blocks | :star: |
| [svelte/no-trailing-spaces](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-trailing-spaces/) | disallow trailing whitespace at the end of lines | :wrench: |

Expand Down
23 changes: 22 additions & 1 deletion docs-svelte-kit/src/lib/components/ESLintCodeBlock.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@
preprocess,
postprocess,
} from "../eslint/scripts/linter.js"
import { loadTsParser } from "../eslint/scripts/ts-parser.js"
import { loadModulesForBrowser } from "../../../../src/shared/svelte-compile-warns/transform/load-module"

const linter = createLinter()
const modulesForBrowser = loadModulesForBrowser()
const loadLinter = createLinter()

let tsParser = null

let code = ""
export let rules = {}
Expand All @@ -19,6 +24,18 @@
preprocess,
postprocess,
}
$: hasLangTs =
/lang\s*=\s*(?:"ts"|ts|'ts'|"typescript"|typescript|'typescript')/u.test(
code,
)
$: linter = modulesForBrowser.then(
hasLangTs && !tsParser
? async () => {
tsParser = await loadTsParser()
return loadLinter
}
: () => loadLinter,
)
let showDiff = fix

function onLintedResult(evt) {
Expand Down Expand Up @@ -48,6 +65,10 @@
parserOptions: {
ecmaVersion: 2020,
sourceType: "module",
parser: {
ts: tsParser,
typescript: tsParser,
},
},
rules,
env: {
Expand Down
3 changes: 2 additions & 1 deletion docs-svelte-kit/src/lib/components/ESLintPlayground.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@
preprocess,
postprocess,
} from "../eslint/scripts/linter.js"
import { loadTsParser } from "../eslint/scripts/ts-parser.js"
import { loadModulesForBrowser } from "../../../../src/shared/svelte-compile-warns/transform/load-module"
let tsParser = null
const linter = loadModulesForBrowser()
.then(async () => {
tsParser = await import("@typescript-eslint/parser")
tsParser = await loadTsParser()
})
.then(() => {
return createLinter()
Expand Down
17 changes: 15 additions & 2 deletions docs-svelte-kit/src/lib/eslint/ESLintEditor.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
lint(linter, code, config, options)
})

let lastResult = {}

async function lint(linter, code, config, options) {
messageMap.clear()
/* eslint-disable no-param-reassign -- ignore */
Expand Down Expand Up @@ -69,12 +71,23 @@
fixedMessages: fixResult.messages,
})

leftMarkers = await Promise.all(
lastResult = { messages, fixResult }

const markers = await Promise.all(
messages.map((m) => messageToMarker(m, messageMap)),
)
rightMarkers = await Promise.all(
const fixedMarkers = await Promise.all(
fixResult.messages.map((m) => messageToMarker(m)),
)
if (
lastResult.messages !== messages ||
lastResult.fixResult !== fixResult
) {
// If the result has changed, don't update the markers
return
}
leftMarkers = markers
rightMarkers = fixedMarkers
}

function applyFix() {
Expand Down
26 changes: 16 additions & 10 deletions docs-svelte-kit/src/lib/eslint/scripts/monaco-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,23 @@ function appendMonacoEditorScript() {
let setupedMonaco = null
let editorLoaded = null

export async function loadMonacoEditor() {
await (setupedMonaco || (setupedMonaco = setupMonaco()))
export function loadMonacoEngine() {
return setupedMonaco || (setupedMonaco = setupMonaco())
}
export function loadMonacoEditor() {
return (
editorLoaded ||
(editorLoaded = new Promise((resolve) => {
if (typeof window !== "undefined") {
// eslint-disable-next-line node/no-missing-require -- ignore
window.require(["vs/editor/editor.main"], (r) => {
resolve(r)
})
}
}))
(editorLoaded = loadModuleFromMonaco("vs/editor/editor.main"))
)
}

export async function loadModuleFromMonaco(moduleName) {
await loadMonacoEngine()
return new Promise((resolve) => {
if (typeof window !== "undefined") {
window.require([moduleName], (r) => {
resolve(r)
})
}
})
}
101 changes: 101 additions & 0 deletions docs-svelte-kit/src/lib/eslint/scripts/ts-create-program.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import type typescript from "typescript"
import type tsvfs from "@typescript/vfs"
type TS = typeof typescript
type TSVFS = typeof tsvfs

/** Create Program */
export function createProgram(
{
ts,
compilerOptions,
compilerHost,
}: {
ts: TS
compilerOptions: typescript.CompilerOptions
compilerHost: typescript.CompilerHost
},
options: { filePath: string },
): typescript.Program {
try {
const program = ts.createProgram({
rootNames: [options.filePath],
options: compilerOptions,
host: compilerHost,
})
return program
} catch (e) {
// eslint-disable-next-line no-console -- Demo debug
console.error(e)
throw e
}
}

export function createCompilerOptions(ts: TS): typescript.CompilerOptions {
const compilerOptions: typescript.CompilerOptions = {
target: ts.ScriptTarget.ESNext,
module: ts.ModuleKind.ESNext,
jsx: ts.JsxEmit.Preserve,
strict: true,
}
compilerOptions.lib = [ts.getDefaultLibFileName(compilerOptions)]
return compilerOptions
}

export async function createVirtualCompilerHost(
{
ts,
tsvfs,
compilerOptions,
}: {
ts: TS
tsvfs: TSVFS
compilerOptions: typescript.CompilerOptions
},
{ filePath: targetFilePath }: { filePath: string },
): Promise<{
compilerHost: typescript.CompilerHost
updateFile: (sourceFile: typescript.SourceFile) => boolean
fsMap: Map<string, string>
}> {
const fsMap = await tsvfs.createDefaultMapFromCDN(
{
lib: Array.from(
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- use internal
(ts as any).libMap.keys(),
),
},
ts.version,
true,
ts,
)
const system = tsvfs.createSystem(fsMap)
const host = tsvfs.createVirtualCompilerHost(system, compilerOptions, ts)
// eslint-disable-next-line @typescript-eslint/unbound-method -- backup original
const original = { getSourceFile: host.compilerHost.getSourceFile }
host.compilerHost.getSourceFile = function (
fileName,
languageVersionOrOptions,
...args
) {
if (targetFilePath === fileName) {
// Exclude the target file from caching as it will be modified.
const file = this.readFile(fileName) ?? ""
return ts.createSourceFile(fileName, file, languageVersionOrOptions, true)
}
if (this.fileExists(fileName)) {
return original.getSourceFile.apply(this, [
fileName,
languageVersionOrOptions,
...args,
])
}
// Avoid error
// eslint-disable-next-line no-console -- Demo debug
console.log(`Not exists: ${fileName}`)
return undefined
}
return {
...host,
fsMap,
}
}
54 changes: 54 additions & 0 deletions docs-svelte-kit/src/lib/eslint/scripts/ts-parser.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { loadMonacoEngine } from "./monaco-loader"
import {
createProgram,
createCompilerOptions,
createVirtualCompilerHost,
} from "./ts-create-program.mts"

let tsParserCache = null
export function loadTsParser() {
return (tsParserCache ??= loadTsParserImpl())
}

async function loadTsParserImpl() {
await loadMonacoEngine()
const [ts, tsvfs, tsParser] = await Promise.all([
import("typescript"),
import("@typescript/vfs"),
import("@typescript-eslint/parser"),
])
if (typeof window === "undefined") {
return tsParser
}
window.define("typescript", ts)

const compilerOptions = createCompilerOptions(ts)
const filePath = "/demo.ts"
const host = await createVirtualCompilerHost(
{ ts, tsvfs, compilerOptions },
{ filePath },
)
return {
parseForESLint(code, options) {
host.fsMap.set(filePath, code)
// Requires its own Program instance to provide full type information.
const program = createProgram(
{ ts, compilerHost: host.compilerHost, compilerOptions },
{ filePath },
)

try {
const result = tsParser.parseForESLint(code, {
...options,
filePath: filePath.replace(/^\//u, ""),
programs: [program],
})
return result
} catch (e) {
// eslint-disable-next-line no-console -- Demo debug
console.error(e)
throw e
}
},
}
}
3 changes: 2 additions & 1 deletion docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,11 @@ These rules relate to style guidelines, and are therefore quite subjective:

## Extension Rules

These rules extend the rules provided by ESLint itself to work well in Svelte:
These rules extend the rules provided by ESLint itself, or other plugins to work well in Svelte:

| Rule ID | Description | |
|:--------|:------------|:---|
| [svelte/@typescript-eslint/no-unnecessary-condition](./rules/@typescript-eslint/no-unnecessary-condition.md) | disallow conditionals where the type is always truthy or always falsy | :wrench: |
| [svelte/no-inner-declarations](./rules/no-inner-declarations.md) | disallow variable or `function` declarations in nested blocks | :star: |
| [svelte/no-trailing-spaces](./rules/no-trailing-spaces.md) | disallow trailing whitespace at the end of lines | :wrench: |

Expand Down
68 changes: 68 additions & 0 deletions docs/rules/@typescript-eslint/no-unnecessary-condition.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
---
pageClass: "rule-details"
sidebarDepth: 0
title: "svelte/@typescript-eslint/no-unnecessary-condition"
description: "disallow conditionals where the type is always truthy or always falsy"
---

# svelte/@typescript-eslint/no-unnecessary-condition

> disallow conditionals where the type is always truthy or always falsy

- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

## :book: Rule Details

This rule extends the base `@typescript-eslint`'s [@typescript-eslint/no-unnecessary-condition] rule.
The [@typescript-eslint/no-unnecessary-condition] rule does not understand reactive or rerendering of Svelte components and has false positives when used with Svelte components. This rule understands reactive and rerendering of Svelte components.

<ESLintCodeBlock fix>

<!--eslint-skip-->

```svelte
<script lang="ts">
/* eslint svelte/@typescript-eslint/no-unnecessary-condition: "error" */
export let foo: number | null = null
/* ✗ BAD */
let b = foo || 42
/* ✓ GOOD */
$: a = foo || 42
</script>

<!-- ✓ GOOD -->
{foo || 42}
```

</ESLintCodeBlock>

## :wrench: Options

```json
{
"@typescript-eslint/no-unnecessary-condition": "off",
"svelte/@typescript-eslint/no-unnecessary-condition": [
"error",
{
"allowConstantLoopConditions": false,
"allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing": false
}
]
}
```

Same as [@typescript-eslint/no-unnecessary-condition] rule option. See [here](https://typescript-eslint.io/rules/no-unnecessary-condition/#options) for details.

## :couple: Related rules

- [@typescript-eslint/no-unnecessary-condition]

[@typescript-eslint/no-unnecessary-condition]: https://typescript-eslint.io/rules/no-unnecessary-condition/

## :mag: Implementation

- [Rule source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/src/rules/@typescript-eslint/no-unnecessary-condition.ts)
- [Test source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/tests/src/rules/@typescript-eslint/no-unnecessary-condition.ts)

<sup>Taken with ❤️ [from @typescript-eslint/eslint-plugin](https://typescript-eslint.io/rules/no-unnecessary-condition/)</sup>
Loading