silvermine / serverless-plugin-external-sns-events

MIT License
26 stars 15 forks source link

eslint not catching coding standards violations #10

Open jthomerson opened 7 years ago

jthomerson commented 7 years ago

In the review I just did (https://github.com/silvermine/serverless-plugin-external-sns-events/pull/8#pullrequestreview-18207943), there were a number of coding standards violations that were not caught. I'm not sure why. Opening the issue here just in case it's a configuration issue with our use of our eslint standards, or the build process, but most likely the real fix will be in one of our eslint repos.

The issues I noted (see the review for all occurrences of them) were:

  1. Indentation
    • There were several places where indentation violated our standards and was not caught. Search the review for "indentation" and you'll see where I commented on them.
  2. Empty object declaration without spaces
    • There was one place where an object initialized with { } (note the space), but our standard is {}. Perhaps we didn't have a rule for this, but I thought we did. Please verify, and add a rule if needed.
  3. Uninitialized variables should go last in var declarations
    • Search the review for "uninitialized variables" and you'll see one place where I commented on this (although it appeared a few places). Our coding standards may not have a rule for this, but should. Please verify, and add a rule if needed.
jimjenkins5 commented 7 years ago

It looks like 1 is only affecting function calls. eslint's indent rule seems to ignore function calls all together unless they contain another node type (i.e. function declaration, object declaration, etc.)

I can right a plugin to cover this, but I'm not sure exactly what we want our standards to be. Here are some options.

// first args on same line as function name, second line indented 3 spaces
fun(arg1, arg2,
   arg3
) {
}

// first args on same line as function name, 
// second line indented to line up with args on first line (similar to our var declaration indentation rule)
fun(arg1, arg2,
    arg3
) {
}

// each arg on separate line, indented 3 spaces
fun(
   arg1,
   arg2,
   arg3
) {
}
jimjenkins5 commented 7 years ago

These rules have been added/fixed in our eslint config and our plugin