Skip to content

Commit

Permalink
Merge pull request #419 from benmosher/issue-415-no-symbols
Browse files Browse the repository at this point in the history
remove for-of syntax from rules
  • Loading branch information
benmosher authored Jul 8, 2016
2 parents 5dd3d70 + f9cf7a0 commit ea84f34
Show file tree
Hide file tree
Showing 11 changed files with 124 additions and 124 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

## [Unreleased]
### Fixed
- pinned `es6-symbol` dependency to `^3.1.0` instead of `*` 😳 ([#415])
- removing `Symbol` dependencies (i.e. `for-of` loops) due to Node 0.10 polyfill
issue (see [#415]). Should not make any discernible semantic difference.

## [1.10.2] - 2016-07-04
### Fixed
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@
"doctrine": "1.2.x",
"es6-map": "^0.1.3",
"es6-set": "^0.1.4",
"es6-symbol": "^3.1.0",
"eslint-import-resolver-node": "^0.2.0",
"lodash.cond": "^4.3.0",
"lodash.endswith": "^4.0.1",
Expand Down
80 changes: 45 additions & 35 deletions src/core/getExports.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import 'es6-symbol/implement'
import Map from 'es6-map'

import * as fs from 'fs'
Expand Down Expand Up @@ -236,18 +235,19 @@ export default class ExportMap {
if (this.reexports.has(name)) return true

// default exports must be explicitly re-exported (#328)
let foundInnerMapName = false
if (name !== 'default') {
for (let dep of this.dependencies.values()) {
let innerMap = dep()
this.dependencies.forEach((dep) => {
if (!foundInnerMapName) {
let innerMap = dep()

// todo: report as unresolved?
if (!innerMap) continue

if (innerMap.has(name)) return true
}
// todo: report as unresolved?
if (innerMap && innerMap.has(name)) foundInnerMapName = true
}
})
}

return false
return foundInnerMapName
}

/**
Expand Down Expand Up @@ -276,24 +276,29 @@ export default class ExportMap {


// default exports must be explicitly re-exported (#328)
let returnValue = { found: false, path: [this] }
if (name !== 'default') {
for (let dep of this.dependencies.values()) {
let innerMap = dep()
// todo: report as unresolved?
if (!innerMap) continue

// safeguard against cycles
if (innerMap.path === this.path) continue

let innerValue = innerMap.hasDeep(name)
if (innerValue.found) {
innerValue.path.unshift(this)
return innerValue
this.dependencies.forEach((dep) => {
if (!returnValue.found) {
let innerMap = dep()
// todo: report as unresolved?
if (innerMap) {

// safeguard against cycles
if (innerMap.path !== this.path) {

let innerValue = innerMap.hasDeep(name)
if (innerValue.found) {
innerValue.path.unshift(this)
returnValue = innerValue
}
}
}
}
}
})
}

return { found: false, path: [this] }
return returnValue
}

get(name) {
Expand All @@ -313,21 +318,26 @@ export default class ExportMap {
}

// default exports must be explicitly re-exported (#328)
let returnValue = undefined
if (name !== 'default') {
for (let dep of this.dependencies.values()) {
let innerMap = dep()
// todo: report as unresolved?
if (!innerMap) continue

// safeguard against cycles
if (innerMap.path === this.path) continue

let innerValue = innerMap.get(name)
if (innerValue !== undefined) return innerValue
}
this.dependencies.foreach((dep) => {
if (returnValue === undefined) {
let innerMap = dep()
// todo: report as unresolved?
if (innerMap) {

// safeguard against cycles
if (innerMap.path !== this.path) {

let innerValue = innerMap.get(name)
if (innerValue !== undefined) returnValue = innerValue
}
}
}
})
}

return undefined
return returnValue
}

forEach(callback, thisArg) {
Expand Down
34 changes: 17 additions & 17 deletions src/core/resolve.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import 'es6-symbol/implement'
import Map from 'es6-map'
import Set from 'es6-set'
import assign from 'object-assign'
Expand Down Expand Up @@ -112,23 +111,24 @@ function fullResolve(modulePath, sourceFile, settings) {

const resolvers = resolverReducer(configResolvers, new Map())

for (let [name, config] of resolvers) {
const resolver = requireResolver(name, sourceFile)
, resolved = withResolver(resolver, config)

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
}
let resolved = { found: false }
resolvers.forEach(function (config, name) {
if (!resolved.found) {
const resolver = requireResolver(name, sourceFile)
resolved = withResolver(resolver, config)
if (resolved.found) {
// resolvers imply file existence, this double-check just ensures the case matches
if (fileExistsWithCaseSync(resolved.path, cacheSettings)) {
// else, counts
cache(resolved.path)
} else {
resolved = { found: false }
}
}
}
})

// failed
// cache(undefined)
return { found: false }
return resolved
}

function resolverReducer(resolvers, map) {
Expand Down
21 changes: 10 additions & 11 deletions src/rules/export.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import 'es6-symbol/implement'
import Map from 'es6-map'
import Set from 'es6-set'

Expand Down Expand Up @@ -32,11 +31,11 @@ module.exports = function (context) {
addNamed(node.declaration.id.name, node.declaration.id)
}

if (node.declaration.declarations != null) {
for (let declaration of node.declaration.declarations) {
recursivePatternCapture(declaration.id, v => addNamed(v.name, v))
}
}
if (node.declaration.declarations == null) return

node.declaration.declarations.forEach(declaration => {
recursivePatternCapture(declaration.id, v => addNamed(v.name, v))
})
},

'ExportAllDeclaration': function (node) {
Expand All @@ -62,15 +61,15 @@ module.exports = function (context) {
},

'Program:exit': function () {
for (let [name, nodes] of named) {
if (nodes.size <= 1) continue
named.forEach((nodes, name) => {
if (nodes.size <= 1) return

for (let node of nodes) {
nodes.forEach(node => {
if (name === 'default') {
context.report(node, 'Multiple default exports.')
} else context.report(node, `Multiple exports of name '${name}'.`)
}
}
})
})
},
}
}
24 changes: 9 additions & 15 deletions src/rules/namespace.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import 'es6-symbol/implement'
import Map from 'es6-map'

import Exports from '../core/getExports'
Expand Down Expand Up @@ -32,7 +31,7 @@ module.exports = function (context) {
return
}

for (let specifier of declaration.specifiers) {
declaration.specifiers.forEach((specifier) => {
switch (specifier.type) {
case 'ImportNamespaceSpecifier':
if (!imports.size) {
Expand All @@ -51,7 +50,7 @@ module.exports = function (context) {
break
}
}
}
})
}
body.forEach(processBodyStatement)
},
Expand Down Expand Up @@ -129,28 +128,23 @@ module.exports = function (context) {

if (pattern.type !== 'ObjectPattern') return

for (let property of pattern.properties) {

pattern.properties.forEach((property) => {
if (property.key.type !== 'Identifier') {
context.report({
node: property,
message: 'Only destructure top-level names.',
})
continue
}

if (!namespace.has(property.key.name)) {
} else if (!namespace.has(property.key.name)) {
context.report({
node: property,
message: makeMessage(property.key, path),
})
continue
} else {
path.push(property.key.name)
testKey(property.value, namespace.get(property.key.name).namespace, path)
path.pop()
}

path.push(property.key.name)
testKey(property.value, namespace.get(property.key.name).namespace, path)
path.pop()
}
})
}

testKey(id, namespaces.get(init.name))
Expand Down
9 changes: 4 additions & 5 deletions src/rules/no-duplicates.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import 'es6-symbol/implement'
import Map from 'es6-map'
import Set from 'es6-set'

import resolve from '../core/resolve'

function checkImports(imported, context) {
for (let [module, nodes] of imported.entries()) {
imported.forEach((nodes, module) => {
if (nodes.size > 1) {
for (let node of nodes) {
nodes.forEach((node) => {
context.report(node, `'${module}' imported multiple times.`)
}
})
}
}
})
}

module.exports = function (context) {
Expand Down
12 changes: 6 additions & 6 deletions src/rules/no-mutable-exports.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ module.exports = function (context) {
}

function checkDeclarationsInScope({variables}, name) {
for (let variable of variables) {
variables.forEach((variable) => {
if (variable.name === name) {
for (let def of variable.defs) {
variable.defs.forEach((def) => {
if (def.type === 'Variable') {
checkDeclaration(def.parent)
}
}
})
}
}
})
}

function handleExportDefault(node) {
Expand All @@ -32,9 +32,9 @@ module.exports = function (context) {
if (node.declaration) {
checkDeclaration(node.declaration)
} else if (!node.source) {
for (let specifier of node.specifiers) {
node.specifiers.forEach((specifier) => {
checkDeclarationsInScope(scope, specifier.local.name)
}
})
}
}

Expand Down
36 changes: 18 additions & 18 deletions src/rules/no-named-as-default-member.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* See LICENSE in root directory for full license.
*/

import 'es6-symbol/implement'
import Map from 'es6-map'

import Exports from '../core/getExports'
Expand Down Expand Up @@ -57,30 +56,31 @@ module.exports = function(context) {
if (!isDestructure) return

const objectName = node.init.name
for (const { key } of node.id.properties) {
if (key == null) continue // true for rest properties
storePropertyLookup(objectName, key.name, key)
}
node.id.properties.forEach(({key}) => {
if (key != null) { // rest properties are null
storePropertyLookup(objectName, key.name, key)
}
})
}

function handleProgramExit() {
allPropertyLookups.forEach((lookups, objectName) => {
const fileImport = fileImports.get(objectName)
if (fileImport == null) return

for (const {propName, node} of lookups) {
if (!fileImport.exportMap.namespace.has(propName)) continue

context.report({
node,
message: (
`Caution: \`${objectName}\` also has a named export ` +
`\`${propName}\`. Check if you meant to write ` +
`\`import {${propName}} from '${fileImport.sourcePath}'\` ` +
'instead.'
),
})
}
lookups.forEach(({propName, node}) => {
if (fileImport.exportMap.namespace.has(propName)) {
context.report({
node,
message: (
`Caution: \`${objectName}\` also has a named export ` +
`\`${propName}\`. Check if you meant to write ` +
`\`import {${propName}} from '${fileImport.sourcePath}'\` ` +
'instead.'
),
})
}
})
})
}

Expand Down
Loading

0 comments on commit ea84f34

Please sign in to comment.