Skip to content

Commit

Permalink
work toward #311, still have a few issues
Browse files Browse the repository at this point in the history
  • Loading branch information
benmosher committed Jul 27, 2016
1 parent d0eaec9 commit 88c4b94
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 67 deletions.
34 changes: 28 additions & 6 deletions src/rules/no-unresolved.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,42 @@
* @author Ben Mosher
*/

import resolve from 'eslint-module-utils/resolve'
import moduleVisitor, { optionsSchema } from 'eslint-module-utils/moduleVisitor'
import resolve, { CASE_SENSITIVE_FS, fileExistsWithCaseSync } from 'eslint-module-utils/resolve'
import ModuleCache from 'eslint-module-utils/ModuleCache'
import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor'

module.exports = function (context) {


exports.meta = {
schema: [ makeOptionsSchema({
caseSensitive: { type: 'boolean', default: true },
})],
}

exports.create = function (context) {

function checkSourceValue(source) {
if (resolve(source.value, context) === undefined) {
const shouldCheckCase = !CASE_SENSITIVE_FS &&
(!context.options[0] || context.options[0].caseSensitive !== false)

const resolvedPath = resolve(source.value, context)

if (resolvedPath === undefined) {
context.report(source,
'Unable to resolve path to module \'' + source.value + '\'.')
`Unable to resolve path to module '${source.value}'.`)
}

else if (shouldCheckCase) {
const cacheSettings = ModuleCache.getSettings(context.settings)
if (!fileExistsWithCaseSync(resolvedPath, cacheSettings)) {
context.report(source,
`Casing of ${source.value} does not match the underlying filesystem.`)
}

}
}

return moduleVisitor(checkSourceValue, context.options[0])

}

module.exports.schema = [ optionsSchema ]
77 changes: 43 additions & 34 deletions tests/src/core/resolve.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { expect } from 'chai'

import resolve, { CASE_SENSITIVE_FS } from 'eslint-module-utils/resolve'
import resolve, { CASE_SENSITIVE_FS, fileExistsWithCaseSync } from 'eslint-module-utils/resolve'
import ModuleCache from 'eslint-module-utils/ModuleCache'

import * as fs from 'fs'
import * as utils from '../utils'
Expand All @@ -26,22 +27,31 @@ describe('resolve', function () {
expect(file).to.equal(utils.testFilePath('./jsx/MyCoolComponent.jsx'))
})

it('should test case sensitivity', function () {
// Note the spelling error 'MyUncoolComponent' vs 'MyUnCoolComponent'
var file = resolve( './jsx/MyUncoolComponent'
, utils.testContext({ 'import/resolve': { 'extensions': ['.jsx'] }})
)

expect(file, 'path to ./jsx/MyUncoolComponent').to.be.undefined
const caseDescribe = (!CASE_SENSITIVE_FS ? describe : describe.skip)
caseDescribe('case sensitivity', function () {
let file
const testContext = utils.testContext({ 'import/resolve': { 'extensions': ['.jsx'] }})
before('resolve', function () {
file = resolve(
// Note the case difference 'MyUncoolComponent' vs 'MyUnCoolComponent'
'./jsx/MyUncoolComponent', testContext)
})
it('resolves regardless of case', function () {
expect(file, 'path to ./jsx/MyUncoolComponent').to.exist
})
it('detects case does not match FS', function () {
expect(fileExistsWithCaseSync(file, ModuleCache.getSettings(testContext)))
.to.be.false
})
})

describe('case cache correctness', function () {
describe('rename cache correctness', function () {
const context = utils.testContext({
'import/cache': { 'lifetime': 1 },
})

const pairs = [
['./CaseyKasem.js', './CASEYKASEM.js'],
['./CaseyKasem.js', './CASEYKASEM2.js'],
]

pairs.forEach(([original, changed]) => {
Expand All @@ -64,33 +74,32 @@ describe('resolve', function () {
utils.testFilePath(changed),
exists => done(exists ? null : new Error('new file does not exist'))))

// these tests fail on a case-sensitive file system
// because nonexistent files aren't cached
if (!CASE_SENSITIVE_FS) {
it('gets cached values within cache lifetime', function () {
// get cached values initially
expect(resolve(original, context)).to.exist
expect(resolve(changed, context)).not.to.exist
})
it('gets cached values within cache lifetime', function () {
// get cached values initially
expect(resolve(original, context)).to.exist
})

it('gets updated values immediately', function () {
// get cached values initially
expect(resolve(changed, context)).to.exist
})

// special behavior for infinity
describe('infinite cache', function () {
this.timeout(1200)
before((done) => setTimeout(done, 1100))

const lifetimes = [ '∞', 'Infinity' ]
lifetimes.forEach(inf => {
const infiniteContext = utils.testContext({
'import/cache': { 'lifetime': inf },
})

// special behavior for infinity
describe('infinite cache', function () {
this.timeout(1200)
before((done) => setTimeout(done, 1100))

const lifetimes = [ '∞', 'Infinity' ]
lifetimes.forEach(inf => {
const infiniteContext = utils.testContext({
'import/cache': { 'lifetime': inf },
})

it(`lifetime: ${inf} still gets cached values after ~1s`, function () {
expect(resolve(original, infiniteContext)).to.exist
expect(resolve(changed, infiniteContext)).not.to.exist
})
it(`lifetime: ${inf} still gets cached values after ~1s`, function () {
expect(resolve(original, infiniteContext), original).to.exist
})
})
}
})

describe('finite cache', function () {
this.timeout(1200)
Expand Down
30 changes: 26 additions & 4 deletions tests/src/rules/no-unresolved.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import * as path from 'path'
import assign from 'object-assign'
import { test, SYNTAX_CASES } from '../utils'

import { CASE_SENSITIVE_FS } from 'eslint-module-utils/resolve'

import { RuleTester } from 'eslint'

var ruleTester = new RuleTester()
Expand Down Expand Up @@ -131,10 +133,6 @@ function runResolverTests(resolver) {
, errors: ["Unable to resolve path to module './does-not-exist'."],
}),

rest({ code: 'import foo from "./jsx/MyUncoolComponent.jsx"'
, errors: ["Unable to resolve path to module './jsx/MyUncoolComponent.jsx'."] }),


// commonjs setting
rest({
code: 'var bar = require("./baz")',
Expand Down Expand Up @@ -204,6 +202,30 @@ function runResolverTests(resolver) {
}),
],
})

if (!CASE_SENSITIVE_FS) {
ruleTester.run('case sensitivity', rule, {
valid: [
rest({ // test with explicit flag
code: 'import foo from "./jsx/MyUncoolComponent.jsx"',
options: [{ caseSensitive: false }],
}),
],

invalid: [
rest({ // test default
code: 'import foo from "./jsx/MyUncoolComponent.jsx"',
errors: [`Casing of ./jsx/MyUncoolComponent.jsx does not match the underlying filesystem.`],
}),
rest({ // test with explicit flag
code: 'import foo from "./jsx/MyUncoolComponent.jsx"',
options: [{ caseSensitive: true }],
errors: [`Casing of ./jsx/MyUncoolComponent.jsx does not match the underlying filesystem.`],
}),
],
})
}

}

['node', 'webpack'].forEach(runResolverTests)
Expand Down
50 changes: 35 additions & 15 deletions utils/moduleVisitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,23 +87,43 @@ exports.default = function visitModules(visitor, options) {
return visitors
}

/**
* make an options schema for the module visitor, optionally
* adding extra fields.
* @param {[type]} additionalProperties [description]
* @return {[type]} [description]
*/
function makeOptionsSchema(additionalProperties) {
const base = {
'type': 'object',
'properties': {
'commonjs': { 'type': 'boolean' },
'amd': { 'type': 'boolean' },
'esmodule': { 'type': 'boolean' },
'ignore': {
'type': 'array',
'minItems': 1,
'items': { 'type': 'string' },
'uniqueItems': true,
},
},
'additionalProperties': false,
}

if (additionalProperties){
for (let key in additionalProperties) {
base.properties[key] = additionalProperties[key]
}
}

return base
}
exports.makeOptionsSchema = makeOptionsSchema

/**
* json schema object for options parameter. can be used to build
* rule options schema object.
* @type {Object}
*/
exports.optionsSchema = {
'type': 'object',
'properties': {
'commonjs': { 'type': 'boolean' },
'amd': { 'type': 'boolean' },
'esmodule': { 'type': 'boolean' },
'ignore': {
'type': 'array',
'minItems': 1,
'items': { 'type': 'string' },
'uniqueItems': true,
},
},
'additionalProperties': false,
}
exports.optionsSchema = makeOptionsSchema()
13 changes: 5 additions & 8 deletions utils/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,24 @@ exports.CASE_SENSITIVE_FS = CASE_SENSITIVE_FS
const fileExistsCache = new ModuleCache()

// http://stackoverflow.com/a/27382838
function fileExistsWithCaseSync(filepath, cacheSettings) {
exports.fileExistsWithCaseSync = function fileExistsWithCaseSync(filepath, cacheSettings) {
// don't care if the FS is case-sensitive
if (CASE_SENSITIVE_FS) return true

// null means it resolved to a builtin
if (filepath === null) return true

const dir = path.dirname(filepath)
const parsedPath = path.parse(filepath)
, dir = parsedPath.dir

let result = fileExistsCache.get(filepath, cacheSettings)
if (result != null) return result

// base case
if (dir === '/' || dir === '.' || /^[A-Z]:\\$/i.test(dir)) {
if (dir === '' || parsedPath.root === filepath) {
result = true
} else {
const filenames = fs.readdirSync(dir)
if (filenames.indexOf(path.basename(filepath)) === -1) {
if (filenames.indexOf(parsedPath.base) === -1) {
result = false
} else {
result = fileExistsWithCaseSync(dir, cacheSettings)
Expand Down Expand Up @@ -102,9 +102,6 @@ function fullResolve(modulePath, sourceFile, settings) {

if (!resolved.found) continue

// resolvers imply file existence, this double-check just ensures the case matches
if (!fileExistsWithCaseSync(resolved.path, cacheSettings)) continue

// else, counts
cache(resolved.path)
return resolved
Expand Down

0 comments on commit 88c4b94

Please sign in to comment.