Merge pull request #598 from nellh/eslint-for-in-guard

Enable eslint guard-for-in to catch for..in vs for..of errors

Author: olgn

.eslintrc.json

@@ -18,6 +18,7 @@
   "rules": {
     "prettier/prettier": "error",
     "no-console": ["error", { "allow": ["warn"] }],
-    "linebreak-style": ["error", "unix"]
+    "linebreak-style": ["error", "unix"],
+    "guard-for-in": "error"
   }
 }

package.json

@@ -12,7 +12,7 @@
   },
   "scripts": {
     "lint": "./node_modules/eslint/bin/eslint.js ./**/*.js",
-    "test": "./node_modules/.bin/mocha tests/ tests/**/**.spec.js",
+    "test": "./node_modules/.bin/mocha tests/ tests/**/**.spec.js '**/__tests__/*.js' --exclude 'node_modules/**/*'",
     "coverage": "./node_modules/.bin/nyc yarn test",
     "codecov": "./node_modules/.bin/codecov",
     "npmPublish": "publish"

validators/__tests__/checkAnyDataPresent.spec.js

@@ -0,0 +1,23 @@
+const assert = require('chai').assert
+const { getFolderSubjects } = require('../checkAnyDataPresent.js')
+
+describe('checkAnyDataPresent', () => {
+  describe('getFolderSubjects()', () => {
+    it('returns only unique subjects', () => {
+      // Native FileList but an array simulates it
+      const fileList = [
+        { relativePath: 'sub-01/files' },
+        { relativePath: 'sub-01/another' },
+        { relativePath: 'sub-02/data' },
+      ]
+      assert.equal(2, getFolderSubjects(fileList).length)
+    })
+    it('filters out emptyroom subject', () => {
+      const fileList = [
+        { relativePath: 'sub-01/files' },
+        { relativePath: 'sub-emptyroom/data' },
+      ]
+      assert.equal(1, getFolderSubjects(fileList).length)
+    })
+  })
+})

validators/checkAnyDataPresent.js

@@ -1,34 +1,29 @@
 var utils = require('../utils')
 var Issue = utils.issues.Issue
 
-function addIfNotPresent(folderSubjects, subject) {
-  if (folderSubjects.indexOf(subject) == -1 && subject !== 'emptyroom') {
-    folderSubjects.push(subject)
-  }
-}
+// Match sub-.../... files, except sub-emptyroom
+const matchSubjectPath = file =>
+  file.relativePath.match(/sub-((?!emptyroom).)*(?=\/)/)
 
-function getFolderSubjects(fileList) {
-  var folderSubjects = []
-  for (var key in fileList) {
-    var file = fileList[key]
-    var match = file.relativePath.match(/sub-(.*?)(?=\/)/)
-    if (match) {
-      // console.log('match:', match)
-      addIfNotPresent(folderSubjects, match[1])
-    }
-  }
-  return folderSubjects
-}
+// Helper for filtering unique values in an array
+const uniqueArray = (value, index, self) => self.indexOf(value) === index
+
+/**
+ * Find unique subjects from FileList
+ * @param {FileList} fileList Browser FileList or Node equivalent
+ */
+const getFolderSubjects = fileList =>
+  Array.from(fileList)
+    .filter(matchSubjectPath)
+    .map(f => matchSubjectPath(f)[1])
+    .filter(uniqueArray)
 
 /**
  * checkAnyDataPresent
  *
  * Takes a list of files and participants with valid data. Checks if they match.
  */
-var checkAnyDataPresent = function checkAnyDataPresent(
-  fileList,
-  summarySubjects,
-) {
+function checkAnyDataPresent(fileList, summarySubjects) {
   var issues = []
   var folderSubjects = getFolderSubjects(fileList)
   var subjectsWithoutAnyValidData = folderSubjects.filter(function(i) {
@@ -55,3 +50,4 @@ var checkAnyDataPresent = function checkAnyDataPresent(
 }
 
 module.exports = checkAnyDataPresent
+module.exports.getFolderSubjects = getFolderSubjects

validators/headerFields.js

@@ -34,14 +34,16 @@ var headerFields = function headerFields(headers) {
     }
   }
 
-  for (file in allIssues39Dict) {
-    var firstIssue = allIssues39Dict[file][0]
-    var evidence = ''
-    for (var issue of allIssues39Dict[file]) {
-      evidence = evidence + ' ' + allIssues39Dict[file][issue].reason
+  for (let file in allIssues39Dict) {
+    if (allIssues39Dict.hasOwnProperty(file)) {
+      const firstIssue = allIssues39Dict[file][0]
+      let evidence = ''
+      for (var issue of allIssues39Dict[file]) {
+        evidence = evidence + ' ' + allIssues39Dict[file][issue].reason
+      }
+      firstIssue.reason = evidence
+      finalIssues.push(firstIssue)
     }
-    firstIssue.reason = evidence
-    finalIssues.push(firstIssue)
   }
 
   return finalIssues
@@ -190,16 +192,18 @@ var headerField = function headerField(headers, field) {
       }
     }
   }
-  for (var nifti_key in nifti_types) {
-    var nifti_type = nifti_types[nifti_key]
-    var max_field_value = Object.keys(nifti_type)[0]
-    for (var field_value_key in nifti_type) {
-      field_value = nifti_type[field_value_key]
-      if (field_value.count > nifti_type[max_field_value].count) {
-        max_field_value = field_value_key
+  for (let nifti_key of Object.keys(nifti_types)) {
+    const nifti_type = nifti_types[nifti_key]
+    let max_field_value = Object.keys(nifti_type)[0]
+    for (let field_value_key in nifti_type) {
+      if (nifti_type.hasOwnProperty(field_value_key)) {
+        field_value = nifti_type[field_value_key]
+        if (field_value.count > nifti_type[max_field_value].count) {
+          max_field_value = field_value_key
+        }
       }
     }
-    for (field_value_key in nifti_type) {
+    for (let field_value_key of Object.keys(nifti_type)) {
       field_value = nifti_type[field_value_key]
       if (
         max_field_value !== field_value_key &&

validators/session.js

@@ -8,63 +8,67 @@ var Issue = utils.issues.Issue
  * directories. Then generates a warning if a given subject is missing any
  * files from the set.
  */
-var session = function missingSessionFiles(fileList) {
-  var subjects = {}
-  var issues = []
-  for (var key in fileList) {
-    var file = fileList[key]
-    var filename
+const session = function missingSessionFiles(fileList) {
+  const subjects = {}
+  const issues = []
+  for (let key in fileList) {
+    if (fileList.hasOwnProperty(key)) {
+      const file = fileList[key]
+      let filename
 
-    if (!file || (typeof window != 'undefined' && !file.webkitRelativePath)) {
-      continue
-    }
+      if (!file || (typeof window != 'undefined' && !file.webkitRelativePath)) {
+        continue
+      }
 
-    var path = file.relativePath
-    if (!utils.type.isBIDS(path) || utils.type.file.isStimuliData(path)) {
-      continue
-    }
-    var subject
-    //match the subject identifier up to the '/' in the full path to a file.
-    var match = path.match(/sub-(.*?)(?=\/)/)
-    if (match === null) {
-      continue
-    } else {
-      subject = match[0]
-    }
+      const path = file.relativePath
+      if (!utils.type.isBIDS(path) || utils.type.file.isStimuliData(path)) {
+        continue
+      }
+      let subject
+      //match the subject identifier up to the '/' in the full path to a file.
+      const match = path.match(/sub-(.*?)(?=\/)/)
+      if (match === null) {
+        continue
+      } else {
+        subject = match[0]
+      }
 
-    // suppress inconsistent subject warnings for sub-emptyroom scans
-    // in MEG data
-    if (subject == 'sub-emptyroom') {
-      continue
-    }
+      // suppress inconsistent subject warnings for sub-emptyroom scans
+      // in MEG data
+      if (subject == 'sub-emptyroom') {
+        continue
+      }
 
-    // initialize an empty array if we haven't seen this subject before
-    if (typeof subjects[subject] === 'undefined') {
-      subjects[subject] = []
+      // initialize an empty array if we haven't seen this subject before
+      if (typeof subjects[subject] === 'undefined') {
+        subjects[subject] = []
+      }
+      // files are prepended with subject name, the following two commands
+      // remove the subject from the file name to allow filenames to be more
+      // easily compared
+      filename = path.substring(path.match(subject).index + subject.length)
+      filename = filename.replace(subject, '<sub>')
+      subjects[subject].push(filename)
     }
-    // files are prepended with subject name, the following two commands
-    // remove the subject from the file name to allow filenames to be more
-    // easily compared
-    filename = path.substring(path.match(subject).index + subject.length)
-    filename = filename.replace(subject, '<sub>')
-    subjects[subject].push(filename)
   }
 
-  var subject_files = []
+  const subject_files = []
 
-  for (var subjKey in subjects) {
-    subject = subjects[subjKey]
-    for (var i = 0; i < subject.length; i++) {
-      file = subject[i]
-      if (subject_files.indexOf(file) < 0) {
-        subject_files.push(file)
+  for (let subjKey in subjects) {
+    if (subjects.hasOwnProperty(subjKey)) {
+      const subject = subjects[subjKey]
+      for (var i = 0; i < subject.length; i++) {
+        const file = subject[i]
+        if (subject_files.indexOf(file) < 0) {
+          subject_files.push(file)
+        }
       }
     }
   }
 
   var subjectKeys = Object.keys(subjects).sort()
   for (var j = 0; j < subjectKeys.length; j++) {
-    subject = subjectKeys[j]
+    const subject = subjectKeys[j]
     for (var set_file = 0; set_file < subject_files.length; set_file++) {
       if (subjects[subject].indexOf(subject_files[set_file]) === -1) {
         var fileThatsMissing =