Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Commit

Permalink
feat: use getConfigForFile to handle configurations
Browse files Browse the repository at this point in the history
Move to using `CLIEngine#getConfigForFile` to handle determination of
the configuration for a file instead of our attempts to replicate this
behavior.

Note that if a user has a configuration in their home directory, and
nowhere more specific for a file, previously we were counting this as
"no configuration" for the purposes of the "Disable when no config"
option. After this change we are doing the same thing ESLint does: Using
this configuration. If this isn't what a user desires, they should move
away from a "global config".

Co-Authored-By: Landon Abney <[email protected]>
  • Loading branch information
aminya and Arcanemagus committed Mar 14, 2021
1 parent 91dfb31 commit fe8adf0
Show file tree
Hide file tree
Showing 20 changed files with 44 additions and 159 deletions.
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,7 @@
"eslint": "4.19.1",
"eslint-rule-documentation": "1.0.23",
"fs-plus": "3.1.1",
"resolve-env": "1.0.0",
"user-home": "2.0.0"
"resolve-env": "1.0.0"
},
"devDependencies": {
"eslint-config-airbnb-base": "13.2.0",
Expand Down
5 changes: 0 additions & 5 deletions spec/fixtures/configs/json/.eslintrc.json

This file was deleted.

1 change: 0 additions & 1 deletion spec/fixtures/configs/json/foo.js

This file was deleted.

8 changes: 0 additions & 8 deletions spec/fixtures/configs/no-ext/.eslintrc

This file was deleted.

1 change: 0 additions & 1 deletion spec/fixtures/configs/no-ext/foo.js

This file was deleted.

1 change: 0 additions & 1 deletion spec/fixtures/configs/none/foo.js

This file was deleted.

1 change: 0 additions & 1 deletion spec/fixtures/configs/package-json/foo.js

This file was deleted.

1 change: 0 additions & 1 deletion spec/fixtures/configs/package-json/nested/foo.js

This file was deleted.

8 changes: 0 additions & 8 deletions spec/fixtures/configs/package-json/nested/package.json

This file was deleted.

11 changes: 0 additions & 11 deletions spec/fixtures/configs/package-json/package.json

This file was deleted.

8 changes: 0 additions & 8 deletions spec/fixtures/configs/yaml/.eslintrc.yaml

This file was deleted.

1 change: 0 additions & 1 deletion spec/fixtures/configs/yaml/foo.js

This file was deleted.

8 changes: 0 additions & 8 deletions spec/fixtures/configs/yml/.eslintrc.yml

This file was deleted.

1 change: 0 additions & 1 deletion spec/fixtures/configs/yml/foo.js

This file was deleted.

2 changes: 1 addition & 1 deletion spec/linter-eslint-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as fs from 'fs'
import { tmpdir } from 'os'
import rimraf from 'rimraf'
// eslint-disable-next-line no-unused-vars
import { beforeEach, it, fit } from 'jasmine-fix'
import { it, fit, wait, beforeEach, afterEach } from 'jasmine-fix'
import linterEslint from '../src/main'

import { processESLintMessages } from '../src/helpers'
Expand Down
68 changes: 24 additions & 44 deletions spec/worker-helpers-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import * as Path from 'path'
import rimraf from 'rimraf'
// eslint-disable-next-line no-unused-vars
import { it, fit, wait, beforeEach, afterEach } from 'jasmine-fix'
import * as Helpers from '../src/worker-helpers'
import { copyFileToTempDir } from './linter-eslint-spec'

Expand Down Expand Up @@ -137,47 +139,27 @@ describe('Worker Helpers', () => {
})
})

describe('getConfigPath', () => {
it('finds .eslintrc', () => {
const fileDir = getFixturesPath(Path.join('configs', 'no-ext'))
const expectedPath = Path.join(fileDir, '.eslintrc')
expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath)
})

it('finds .eslintrc.yaml', () => {
const fileDir = getFixturesPath(Path.join('configs', 'yaml'))
const expectedPath = Path.join(fileDir, '.eslintrc.yaml')
expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath)
})

it('finds .eslintrc.yml', () => {
const fileDir = getFixturesPath(Path.join('configs', 'yml'))
const expectedPath = Path.join(fileDir, '.eslintrc.yml')
expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath)
})
describe('getConfigForFile', () => {
// Use the bundled ESLint for the tests
const eslint = require('eslint')
const fixtureFile = getFixturesPath(Path.join('configs', 'js', 'foo.js'))

it('finds .eslintrc.js', () => {
const fileDir = getFixturesPath(Path.join('configs', 'js'))
const expectedPath = Path.join(fileDir, '.eslintrc.js')
expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath)
it('uses ESLint to determine the configuration', () => {
const filePath = fixtureFile
const foundConfig = Helpers.getConfigForFile(eslint, filePath)
expect(foundConfig.rules.semi).toEqual([2, 'never'])
})

it('finds .eslintrc.json', () => {
const fileDir = getFixturesPath(Path.join('configs', 'json'))
const expectedPath = Path.join(fileDir, '.eslintrc.json')
expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath)
})
it('returns null when the file has no configuration', async () => {
// Copy the file to a temporary folder
const filePath = await copyFileToTempDir(fixtureFile)
const tempDir = Path.dirname(filePath)

it('finds package.json with an eslintConfig property', () => {
const fileDir = getFixturesPath(Path.join('configs', 'package-json'))
const expectedPath = Path.join(fileDir, 'package.json')
expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath)
})
const foundConfig = Helpers.getConfigForFile(eslint, filePath)
expect(foundConfig).toBeNull()

it('ignores package.json with no eslintConfig property', () => {
const fileDir = getFixturesPath(Path.join('configs', 'package-json', 'nested'))
const expectedPath = getFixturesPath(Path.join('configs', 'package-json', 'package.json'))
expect(Helpers.getConfigPath(fileDir)).toBe(expectedPath)
// Remove the temporary directory
rimraf.sync(tempDir)
})
})

Expand All @@ -204,13 +186,12 @@ describe('Worker Helpers', () => {
it('returns the path relative to the project dir if provided when no ignore file is found', async () => {
const fixtureFile = getFixturesPath(Path.join('files', 'good.js'))
// Copy the file to a temporary folder
const tempFixturePath = await copyFileToTempDir(fixtureFile)
const tempDir = Path.dirname(tempFixturePath)
const filepath = Path.join(tempDir, 'good.js')
const filePath = await copyFileToTempDir(fixtureFile)
const tempDir = Path.dirname(filePath)
const tempDirParent = Path.dirname(tempDir)
const config = createConfig()

const relativePath = Helpers.getRelativePath(tempDir, filepath, config, tempDirParent)
const relativePath = Helpers.getRelativePath(tempDir, filePath, config, tempDirParent)
// Since the project is the parent of the temp dir, the relative path should be
// the dir containing the file, plus the file. (e.g. asgln3/good.js)
const expectedPath = Path.join(Path.basename(tempDir), 'good.js')
Expand All @@ -222,12 +203,11 @@ describe('Worker Helpers', () => {
it('returns just the file being linted if no ignore file is found and no project dir is provided', async () => {
const fixtureFile = getFixturesPath(Path.join('files', 'good.js'))
// Copy the file to a temporary folder
const tempFixturePath = await copyFileToTempDir(fixtureFile)
const tempDir = Path.dirname(tempFixturePath)
const filepath = Path.join(tempDir, 'good.js')
const filePath = await copyFileToTempDir(fixtureFile)
const tempDir = Path.dirname(filePath)
const config = createConfig()

const relativePath = Helpers.getRelativePath(tempDir, filepath, config, null)
const relativePath = Helpers.getRelativePath(tempDir, filePath, config, null)
expect(relativePath).toBe('good.js')

// Remove the temporary directory
Expand Down
18 changes: 0 additions & 18 deletions src/is-config-at-home-root.js

This file was deleted.

26 changes: 0 additions & 26 deletions src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,13 @@ const idleCallbacks = new Set()
// Dependencies
// NOTE: We are not directly requiring these in order to reduce the time it
// takes to require this file as that causes delays in Atom loading this package
let path
let helpers
let workerHelpers
let isConfigAtHomeRoot
let migrateConfigOptions

const loadDeps = () => {
if (!path) {
path = require('path')
}
if (!helpers) {
helpers = require('./helpers')
}
if (!workerHelpers) {
workerHelpers = require('./worker-helpers')
}
if (!isConfigAtHomeRoot) {
isConfigAtHomeRoot = require('./is-config-at-home-root')
}
}

const makeIdleCallback = (work) => {
Expand Down Expand Up @@ -64,7 +52,6 @@ let showRule
let lintHtmlFiles
let ignoredRulesWhenModified
let ignoredRulesWhenFixing
let disableWhenNoEslintConfig
let ignoreFixableRulesWhileTyping

// Internal functions
Expand Down Expand Up @@ -148,11 +135,6 @@ module.exports = {
(value) => { showRule = value }
))

this.subscriptions.add(atom.config.observe(
'linter-eslint.disabling.disableWhenNoEslintConfig',
(value) => { disableWhenNoEslintConfig = value }
))

this.subscriptions.add(atom.config.observe(
'linter-eslint.disabling.rulesToSilenceWhileTyping',
(ids) => { ignoredRulesWhenModified = ids }
Expand Down Expand Up @@ -291,7 +273,6 @@ module.exports = {
}

const filePath = textEditor.getPath()
const fileDir = path.dirname(filePath)
const projectPath = atom.project.relativizePath(filePath)[0]

// Get the text from the editor, so we can use executeOnText
Expand All @@ -301,13 +282,6 @@ module.exports = {
return
}

// Do not try to fix if linting should be disabled
const configPath = workerHelpers.getConfigPath(fileDir)
const noProjectConfig = (configPath === null || isConfigAtHomeRoot(configPath))
if (noProjectConfig && disableWhenNoEslintConfig) {
return
}

let rules = {}
if (Object.keys(ignoredRulesWhenFixing).length > 0) {
rules = ignoredRulesWhenFixing
Expand Down
22 changes: 14 additions & 8 deletions src/worker-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ export function getESLintInstance(fileDir, config, projectPath) {
return getESLintFromDirectory(modulesDir, config, projectPath)
}

export function getConfigForFile(eslint, filePath) {
const cli = new eslint.CLIEngine()
try {
return cli.getConfigForFile(filePath)
} catch (e) {
// No configuration was found
return null
}
}

// Unused function
export function getConfigPath(fileDir) {
const configFile = findCached(fileDir, [
'.eslintrc.js', '.eslintrc.yaml', '.eslintrc.yml', '.eslintrc.json', '.eslintrc', 'package.json'
Expand Down Expand Up @@ -161,27 +172,22 @@ export function getRelativePath(fileDir, filePath, config, projectPath) {
return Path.basename(filePath)
}

export function getCLIEngineOptions(type, config, rules, filePath, fileDir, givenConfigPath) {
export function getCLIEngineOptions(type, config, rules, filePath, fileConfig) {
const cliEngineConfig = {
rules,
ignore: !config.advanced.disableEslintIgnore,
fix: type === 'fix'
}

const ignoreFile = config.advanced.disableEslintIgnore ? null : findCached(fileDir, '.eslintignore')
if (ignoreFile) {
cliEngineConfig.ignorePath = ignoreFile
}

cliEngineConfig.rulePaths = config.advanced.eslintRulesDirs.map((path) => {
const rulesDir = cleanPath(path)
if (!Path.isAbsolute(rulesDir)) {
return findCached(fileDir, rulesDir)
return findCached(Path.dirname(filePath), rulesDir)
}
return rulesDir
}).filter(path => path)

if (givenConfigPath === null && config.global.eslintrcPath) {
if (fileConfig === null && config.global.eslintrcPath) {
// If we didn't find a configuration use the fallback from the settings
cliEngineConfig.configFile = cleanPath(config.global.eslintrcPath)
}
Expand Down
9 changes: 4 additions & 5 deletions src/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import Path from 'path'
import { FindCache, findCached } from 'atom-linter'
import * as Helpers from './worker-helpers'
import isConfigAtHomeRoot from './is-config-at-home-root'

process.title = 'linter-eslint helper'

Expand Down Expand Up @@ -50,17 +49,17 @@ module.exports = async () => {

const fileDir = Path.dirname(filePath)
const eslint = Helpers.getESLintInstance(fileDir, config, projectPath)
const configPath = Helpers.getConfigPath(fileDir)
const noProjectConfig = (configPath === null || isConfigAtHomeRoot(configPath))
if (noProjectConfig && config.disabling.disableWhenNoEslintConfig) {

const fileConfig = Helpers.getConfigForFile(eslint, filePath)
if (fileConfig === null && config.disabling.disableWhenNoEslintConfig) {
emit(emitKey, { messages: [] })
return
}

const relativeFilePath = Helpers.getRelativePath(fileDir, filePath, config, projectPath)

const cliEngineOptions = Helpers
.getCLIEngineOptions(type, config, rules, relativeFilePath, fileDir, configPath)
.getCLIEngineOptions(type, config, rules, relativeFilePath, fileConfig)

let response
if (type === 'lint') {
Expand Down

0 comments on commit fe8adf0

Please sign in to comment.