Fix documentation lint issues

[rmloveland]

Type of change

• Bugfix
• Feature
• New bidder adapter
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Other

Description of change

Modify prebid.js to address issues found by running the documentation lint command.

In particular I needed to change the variable name from cmd to command in the function starting on this line in order to address the fact that using cmd as the variable name and as the array on the global object was confusing JSDoc.

https://github.com/prebid/Prebid.js/compare/fix-documentation-lint-issues?expand=1#diff-f8e049a1b6d4f9aa96fca41d2f7aa11dR781

I ran gulp test and everything passed, but I wanted to call that out just in case.

Other information

Next baby step towards #1408

[dbemiller]

Changes look good to me.

One thing I'll note is that I don't think documentation lint is running on folders recursively. So there are likely to be lots of other errors in modules, src, etc.

No reason that that should hold up these changes, though.

[dbemiller]

I'm not positive, but... is this generating the right docs? I wouldn't expect to see @property used in this way.

A sample call would look something like:

pbjs.requestBids({
  timeout: ...,
  adUnits: ...,
  adUnitCodes: ...,
  bidsBackHandler: function(bids) { ... }
});

If so, then cool ^^.

[rmloveland]

I was trying to figure that out - mostly I wanted to shut up the linter. I don't think (?) it's generating the right docs - it's generating the following:

### requestBids

**Parameters**

-   `$0` **[Object](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object)**  (optional, default `{}`)
    -   `$0.bidsBackHandler`  
    -   `$0.timeout`  
    -   `$0.adUnits`  
    -   `$0.adUnitCodes`  
-   `bidsBackHandler`  
-   `timeout`  
-   `adUnits`  
-   `adUnitCodes`  

I don't know if that is the right doc structure b/c I don't understand what this way of referring to arguments means:

requestBids = function ({ bidsBackHandler, timeout, adUnits, adUnitCodes } = {})

Does it mean that:

1. This defaults to an empty object?
2. That providing the object with the right keys as input is optional? Because it doesn't seem optional. (?)
3. Something else?

[dbemiller]

I tested a bit... I think what you want is to swap the property tags here with param. That makes more reasonable docs (screenshotted below)

See also: http://usejsdoc.org/tags-param.html#parameters-with-properties

[rmloveland]

Ah, makes sense after reading the doc. OK, just switched back to param.

After changing back to param, documentation lint still throwing one warning, but the message reads like its parser just isn't smart enough to understand the code/JSDoc, not like there is an actual problem, so it seems like we can safely ignore it:

warning  An explicit parameter named defaults was specified but didn't match inferred information $0

[dbemiller]

oh that's super annoying...

That sounds like an error meant to prevent mismatched names, like this:

/**
 * @param e ...
 */
function handleError(error) { ... }

I looked around a bit, and found this: documentationjs/documentation#270

Sounds like $0 is just the placeholder value they assign because the fancy ES6 syntax doesn't actually give the param a name.

[mkendall07]

should we call this something else? Like requestOptions or something?

[rmloveland]

Good point - changed to requestOptions.

[mkendall07]

so confused with the lowercase string and uppercase Object

[rmloveland]

I know! The change is based on the output of documentation lint, and also in the Google Closure Compiler JSDoc guidelines FWIW.

[dbemiller]

that's why linters exist in the first place :).

Sometimes it's for consistency. Sometimes it's because your code is subtly incorrect.

My guess is that they were expecting to support Classes in the newer versions of JS, and the general principle was "primitives are lowercase. Classes are uppercase." Just speculation though.