Skip to content

Commit

Permalink
feat: add support for ignoring sync methods from certain locations
Browse files Browse the repository at this point in the history
  • Loading branch information
RebeccaStevens committed Nov 28, 2024
1 parent ccf5f9e commit d27b2a8
Show file tree
Hide file tree
Showing 7 changed files with 303 additions and 5 deletions.
57 changes: 57 additions & 0 deletions docs/rules/no-sync.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ fs.readFileSync(somePath).toString();
#### ignores

You can `ignore` specific function names using this option.
Additionally, if you are using TypeScript you can optionally specify where the function is declared.

Examples of **incorrect** code for this rule with the `{ ignores: ['readFileSync'] }` option:

Expand All @@ -78,6 +79,62 @@ Examples of **correct** code for this rule with the `{ ignores: ['readFileSync']
fs.readFileSync(somePath);
```

##### Advanced (TypeScript only)

You can provide a list of specifiers to ignore. Specifiers are typed as follows:

```ts
type Specifier =
| string
| {
from: "file";
path?: string;
name?: string[];
}
| {
from: "package";
package?: string;
name?: string[];
}
| {
from: "lib";
name?: string[];
}
```
###### From a file
Examples of **correct** code for this rule with the ignore file specifier:
```js
/*eslint n/no-sync: ["error", { ignores: [{ from: 'file', path: './foo.ts' }]}] */

import { fooSync } from "./foo"
fooSync()
```

###### From a package

Examples of **correct** code for this rule with the ignore package specifier:

```js
/*eslint n/no-sync: ["error", { ignores: [{ from: 'package', package: 'effect' }]}] */

import { Effect } from "effect"
const value = Effect.runSync(Effect.succeed(42))
```

###### From the TypeScript library

Examples of **correct** code for this rule with the ignore lib specifier:

```js
/*eslint n/no-sync: ["error", { ignores: [{ from: 'lib' }]}] */

const stylesheet = new CSSStyleSheet()
stylesheet.replaceSync("body { font-size: 1.4em; } p { color: red; }")
```

## 🔎 Implementation

- [Rule source](../../lib/rules/no-sync.js)
Expand Down
91 changes: 88 additions & 3 deletions lib/rules/no-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
*/
"use strict"

const typeMatchesSpecifier = require("ts-declaration-location")
const getTypeOfNode = require("../util/get-type-of-node")
const getParserServices = require("../util/get-parser-services")

const selectors = [
// fs.readFileSync()
// readFileSync.call(null, 'path')
Expand Down Expand Up @@ -32,7 +36,65 @@ module.exports = {
},
ignores: {
type: "array",
items: { type: "string" },
items: {
oneOf: [
{ type: "string" },
{
type: "object",
properties: {
from: {
type: "string",
enum: ["file"],
},
path: {
type: "string",
},
name: {
type: "array",
items: {
type: "string",
},
},
},
additionalProperties: false,
},
{
type: "object",
properties: {
from: {
type: "string",
enum: ["lib"],
},
name: {
type: "array",
items: {
type: "string",
},
},
},
additionalProperties: false,
},
{
type: "object",
properties: {
from: {
type: "string",
enum: ["package"],
},
package: {
type: "string",
},
name: {
type: "array",
items: {
type: "string",
},
},
},
additionalProperties: false,
},
],
},
default: [],
},
},
Expand All @@ -57,8 +119,31 @@ module.exports = {
* @returns {void}
*/
[selector.join(",")](node) {
if (ignores.includes(node.name)) {
return
const parserServices = getParserServices(context)

for (const ignore of ignores) {
if (typeof ignore === "string") {
if (ignore === node.name) {
return
}

continue
}

if (parserServices !== null) {
const type = getTypeOfNode(node, parserServices)
if (
typeMatchesSpecifier(

Check failure on line 136 in lib/rules/no-sync.js

View workflow job for this annotation

GitHub Actions / Lint

This expression is not callable.
parserServices.program,
ignore,
type
) &&
(ignore.name === undefined ||
ignore.name.includes(node.name))
) {
return
}
}
}

context.report({
Expand Down
24 changes: 24 additions & 0 deletions lib/util/get-parser-services.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
"use strict"

const {
getParserServices: getParserServicesFromTsEslint,
} = require("@typescript-eslint/utils/eslint-utils")

/**
* Get the TypeScript parser services.
* If TypeScript isn't present, returns `null`.
*
* @param {import('eslint').Rule.RuleContext} context - rule context
* @returns {import('@typescript-eslint/parser').ParserServices | null}
*/
module.exports = function getParserServices(context) {
// Not using tseslint parser?
if (
context.sourceCode.parserServices?.esTreeNodeToTSNodeMap == null ||
context.sourceCode.parserServices.tsNodeToESTreeNodeMap == null
) {
return null
}

return getParserServicesFromTsEslint(context, true)

Check failure on line 23 in lib/util/get-parser-services.js

View workflow job for this annotation

GitHub Actions / Lint

No overload matches this call.
}
21 changes: 21 additions & 0 deletions lib/util/get-type-of-node.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
"use strict"

/**
* Get the type of a node.
* If TypeScript isn't present, returns `null`.
*
* @param {import('estree').Node} node - A node
* @param {import('@typescript-eslint/parser').ParserServices} parserServices - A parserServices
* @returns {import('typescript').Type | null}
*/
module.exports = function getTypeOfNode(node, parserServices) {
const { esTreeNodeToTSNodeMap, program } = parserServices
if (program === null) {
return null
}
const tsNode = esTreeNodeToTSNodeMap.get(node)

Check failure on line 16 in lib/util/get-type-of-node.js

View workflow job for this annotation

GitHub Actions / Lint

Argument of type 'import("/home/runner/work/eslint-plugin-n/eslint-plugin-n/node_modules/@types/estree/index").Node' is not assignable to parameter of type 'import("/home/runner/work/eslint-plugin-n/eslint-plugin-n/node_modules/@typescript-eslint/types/dist/generated/ast-spec").Node'.
const checker = program.getTypeChecker()
const nodeType = checker.getTypeAtLocation(tsNode)
const constrained = checker.getBaseConstraintOfType(nodeType)
return constrained ?? nodeType
}
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@
},
"dependencies": {
"@eslint-community/eslint-utils": "^4.4.1",
"@typescript-eslint/utils": "^8.16.0",
"enhanced-resolve": "^5.17.1",
"eslint-plugin-es-x": "^7.8.0",
"get-tsconfig": "^4.8.1",
"globals": "^15.11.0",
"ignore": "^5.3.2",
"minimatch": "^9.0.5",
"semver": "^7.6.3"
"semver": "^7.6.3",
"ts-declaration-location": "^1.0.5"
},
"devDependencies": {
"@eslint/js": "^9.14.0",
Expand Down
91 changes: 90 additions & 1 deletion tests/lib/rules/no-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
"use strict"

const RuleTester = require("#test-helpers").RuleTester
const { RuleTester, TsRuleTester } = require("#test-helpers")
const rule = require("../../../lib/rules/no-sync")

new RuleTester().run("no-sync", rule, {
Expand Down Expand Up @@ -149,3 +149,92 @@ new RuleTester().run("no-sync", rule, {
},
],
})

new TsRuleTester().run("no-sync", rule, {
valid: [
{
code: `
declare function fooSync(): void;
fooSync();
`,
filename: "foo.ts",
options: [
{
ignores: [
{
from: "file",
path: "**/*.ts",
},
],
},
],
},
{
code: `
declare function fooSync(): void;
fooSync();
`,
filename: "foo.ts",
options: [
{
ignores: [
{
from: "file",
path: "**/*.ts",
name: ["fooSync"],
},
],
},
],
},
],
invalid: [
{
code: `
declare function fooSync(): void;
fooSync();
`,
filename: "foo.ts",
options: [
{
ignores: [
{
from: "file",
path: "./bar.ts",
},
],
},
],
errors: [
{
messageId: "noSync",
data: { propertyName: "fooSync" },
type: "CallExpression",
},
],
},
{
code: `
declare function fooSync(): void;
fooSync();
`,
options: [
{
ignores: [
{
from: "file",
name: ["barSync"],
},
],
},
],
errors: [
{
messageId: "noSync",
data: { propertyName: "fooSync" },
type: "CallExpression",
},
],
},
],
})
20 changes: 20 additions & 0 deletions tests/test-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const { FlatRuleTester } = require("eslint/use-at-your-own-risk")
const globals = require("globals")
const semverSatisfies = require("semver/functions/satisfies")
const os = require("os")
const typescriptParser = require("@typescript-eslint/parser")

// greater than or equal to ESLint v9
exports.gteEslintV9 = semverSatisfies(eslintVersion, ">=9", {
Expand All @@ -33,6 +34,18 @@ const defaultConfig = {
globals: { ...globals.es2015, ...globals.node },
},
}
const tsConfig = {
languageOptions: {
ecmaVersion: "latest",
sourceType: "module",
parser: typescriptParser,
parserOptions: {
projectService: {
allowDefaultProject: ["*.*", "src/*"], // TODO: Don't use default project.
},
},
},
}
exports.RuleTester = function (config = defaultConfig) {
if (config.languageOptions.env?.node === false)
config.languageOptions.globals = config.languageOptions.globals || {}
Expand All @@ -54,6 +67,13 @@ exports.RuleTester = function (config = defaultConfig) {
}
return ruleTester
}
exports.TsRuleTester = function (config = tsConfig) {
return exports.RuleTester.call(this, config)
}
Object.setPrototypeOf(
exports.TsRuleTester.prototype,
exports.RuleTester.prototype
)

// support skip in tests
function shouldRun(item) {
Expand Down

0 comments on commit d27b2a8

Please sign in to comment.