Merge pull request #52 from poiru/fix-is-optional

Fix utils.isOptional()

Author: zeke

lib/module-declaration.js

@@ -66,7 +66,7 @@ const generateModuleDeclaration = (module, index, API) => {
           newType = utils.genMethodString(paramInterfaces, module, eventListenerArg, eventListenerArg.parameters, null, true)
         }
 
-        args.push(`${argString}${utils.paramify(eventListenerArg.name)}${utils.isOptional(eventListenerArg, false) ? '?' : ''}: ${newType}`)
+        args.push(`${argString}${utils.paramify(eventListenerArg.name)}${utils.isOptional(eventListenerArg) ? '?' : ''}: ${newType}`)
       })
       listener = `(${args.join(`,\n${indent}`)}) => void`
     }

lib/utils.js

@@ -116,30 +116,16 @@ const isEmitter = (module) => {
       return true
   }
 }
-const isOptional = (param, defaultReturn) => {
-  if (typeof defaultReturn === 'undefined') {
-    defaultReturn = true
-  }
-  // Does the description contain the word "optional"?
-  if (/optional/i.test(param.description)) {
-    return true
-  }
-
+const isOptional = (param) => {
   // Did we pass a "required"?
   if (typeof param.required !== 'undefined') {
     return !param.required
   }
 
-  // Does the description not contain the word "required"?
-  if (param.description && !/required/i.test(param.description) && /\(.+\)/.test(param.description)) {
-    return true
-  }
-
-  // Let's be optimistic - having optional props marked as required
-  // ruins the developer experience, why a false negative is only
-  // slightly annoying
+  // Assume that methods are never optional because electron-docs-linter
+  // doesn't currently mark them as required.
   debug(`Could not determine optionality for ${param.name}`)
-  return defaultReturn
+  return param.type !== 'Function'
 }
 
 const genMethodString = (paramInterfaces, module, moduleMethod, parameters, returns, includeType, paramTypePrefix) => {

test/utils_spec.js

@@ -100,20 +100,20 @@ describe('utils', () => {
   })
 
   describe('isOptional', () => {
-    it('should return false for an empty description', () => {
-      expect(utils.isOptional({ description: '', required: true })).to.eq(false)
+    it('should return true if param is not required', () => {
+      expect(utils.isOptional({})).to.eq(true)
     })
 
-    it('should return true if optional is in the description', () => {
-      expect(utils.isOptional({ description: 'This param is completely optional' })).to.eq(true)
+    it('should return false if param is required', () => {
+      expect(utils.isOptional({ required: true })).to.eq(false)
     })
 
-    it('should return true if the description has brackets and is not required', () => {
-      expect(utils.isOptional({ description: '(not needed) - This thing' })).to.eq(true)
+    it('should default to true if param is a non-function', () => {
+      expect(utils.isOptional({ type: 'Foo' })).to.eq(true)
     })
 
-    it('should return false if the description has brackets but is required', () => {
-      expect(utils.isOptional({ description: '(not needed) - This thing is required', required: true })).to.eq(false)
+    it('should default to false if param is a function', () => {
+      expect(utils.isOptional({ type: 'Function' })).to.eq(false)
     })
   })
 })