Added 2 (+/-1) sentence rationales for 2/3 of rules (palantir#3734)

* Added a couple extra docs lines to align

* Added a couple extra docs lines to arrow-return-shorthand

* Added a couple extra docs lines to await-promise

* Added a couple extra docs lines to binary-expression-operand-order

* Added a couple extra docs to class-name

* Added a couple extra docs to completed-docs

* Added another doc line to cyclomatic-complexity

* Switch link to https in eofline

* Added a couple extra docs to for-in

* Added a couple extra docs to member-access

* Added a couple extra docs to member-ordering

* Added a couple extra docs to no-any

* Added a couple extra docs to no-boolean-literal-compare

* Added a couple extra docs to no-consecutive-blank-lines

* Added a couple extra docs to no-dynamic-delete

* Added a couple extra docs to no-floating-promises

* Added a couple extra docs to no-inferred-empty-object-type

* Added a brief rationale to no-invalid-template-strings-rule

* Typo correction on no-magic-numbers

* Added a couple extra docs to no-misused-new

* Added a couple extra docs to no-non-null-assertion

* Added a couple extra docs to no-null-keyword

* Added a couple extra docs to no-parameter-properties

* Added a couple extra docs to no-shadowed-variable

* Added a couple extra docs to no-string-throw

* Added a couple extra docs to no-this-assignment

* Added a couple extra docs to no-unbound-method

* Added a couple extra docs to no-unnecessary-callback-wrapper

* Added a couple extra docs to no-unnecessary-initializer

* Added a couple extra docs to no-unsafe-any

* Added a couple extra docs to no-unused-variable

* Added a couple extra docs to no-var-keyword

* Added a couple extra docs to no-var-requires

* Added a couple extra docs to no-void-expression

* Added a couple extra docs to number-literal-format

* Missing dedent in no-non-null-assertion

src/rules/alignRule.ts

@@ -40,7 +40,11 @@ export class Rule extends Lint.Rules.AbstractRule {
         ruleName: "align",
         description: "Enforces vertical alignment.",
         hasFix: true,
-        rationale: "Helps maintain a readable, consistent style in your codebase.",
+        rationale: Lint.Utils.dedent`
+            Helps maintain a readable, consistent style in your codebase.
+
+            Consistent alignment for code statements helps keep code readable and clear.
+            Statements misaligned from the standard can be harder to read and understand.`,
         optionsDescription: Lint.Utils.dedent`
             Five arguments may be optionally provided:
 

src/rules/arrowReturnShorthandRule.ts

@@ -39,6 +39,10 @@ export class Rule extends Lint.Rules.AbstractRule {
             true,
             [true, OPTION_MULTILINE],
         ],
+        rationale: Lint.Utils.dedent`
+            It's unnecessary to include \`return\` and \`{}\` brackets in arrow lambdas.
+            Leaving them out results in simpler and easier to read code.
+        `,
         type: "style",
         typescriptOnly: false,
     };

src/rules/awaitPromiseRule.ts

@@ -40,8 +40,11 @@ export class Rule extends Lint.Rules.TypedRule {
         },
         optionExamples: [true, [true, "Thenable"]],
         rationale: Lint.Utils.dedent`
-            While it is valid TypeScript to await a non-Promise-like value (it will resolve immediately),
+            While it is valid JavaScript to await a non-Promise-like value (it will resolve immediately),
             this pattern is often a programmer error and the resulting semantics can be unintuitive.
+
+            Awaiting non-Promise-like values often is an indication of programmer error, such as
+            forgetting to add parenthesis to call a function that returns a Promise.
         `,
         type: "functionality",
         typescriptOnly: true,

src/rules/binaryExpressionOperandOrderRule.ts

@@ -31,6 +31,12 @@ export class Rule extends Lint.Rules.AbstractRule {
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],
+        rationale: Lint.Utils.dedent`
+            Expressions like \`1 + x\` are sometimes referred to as "Yoda" expressions because they read
+            opposite to how we would normally speak the expression.
+
+            Sticking to a consistent grammar for conditions helps keep code readable and understandable.
+        `,
         type: "style",
         typescriptOnly: false,
     };

src/rules/classNameRule.ts

@@ -26,7 +26,12 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "class-name",
         description: "Enforces PascalCased class and interface names.",
-        rationale: "Makes it easy to differentiate classes from regular variables at a glance.",
+        rationale: Lint.Utils.dedent`
+            Makes it easy to differentiate classes from regular variables at a glance.
+
+            JavaScript and general programming convention is to refer to classes in PascalCase.
+            It's confusing to use camelCase or other conventions for class names.
+        `,
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],

src/rules/completedDocsRule.ts

@@ -280,7 +280,12 @@ export class Rule extends Lint.Rules.TypedRule {
                 },
             ],
         ],
+        rationale: Lint.Utils.dedent`
+            Helps ensure important components are documented.
 
+            Note: use this rule sparingly. It's better to have self-documenting names on components with single, consice responsibilities.
+            Comments that only restate the names of variables add nothing to code, and can easily become outdated.
+        `,
         type: "style",
         typescriptOnly: false,
         requiresTypeInfo: true,

src/rules/cyclomaticComplexityRule.ts

@@ -42,7 +42,9 @@ export class Rule extends Lint.Rules.AbstractRule {
         rationale: Lint.Utils.dedent`
             Cyclomatic complexity is a code metric which indicates the level of complexity in a
             function. High cyclomatic complexity indicates confusing code which may be prone to
-            errors or difficult to modify.`,
+            errors or difficult to modify.
+
+            It's better to have smaller, single-purpose functions with self-documenting names.`,
         optionsDescription: Lint.Utils.dedent`
             An optional upper limit for cyclomatic complexity can be specified. If no limit option
             is provided a default value of ${Rule.DEFAULT_THRESHOLD} will be used.`,

src/rules/eoflineRule.ts

@@ -25,7 +25,7 @@ export class Rule extends Lint.Rules.AbstractRule {
         ruleName: "eofline",
         description: "Ensures the file ends with a newline.",
         descriptionDetails: "Fix for single-line files is not supported.",
-        rationale: "It is a [standard convention](http://stackoverflow.com/q/729692/3124288) to end files with a newline.",
+        rationale: "It is a [standard convention](https://stackoverflow.com/q/729692/3124288) to end files with a newline.",
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],

src/rules/forinRule.ts

@@ -35,7 +35,13 @@ export class Rule extends Lint.Rules.AbstractRule {
             \`\`\`
             Prevents accidental iteration over properties inherited from an object's prototype.
             See [MDN's \`for...in\`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...in)
-            documentation for more information about \`for...in\` loops.`,
+            documentation for more information about \`for...in\` loops.
+
+            Also consider using a [\`Map\`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map)
+            or [\`Set\`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set)
+            if you're storing collections of objects.
+            Using \`Object\`s can cause occasional edge case bugs, such as if a key is named "hasOwnProperty".
+        `,
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],

src/rules/memberAccessRule.ts

@@ -47,7 +47,12 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "member-access",
         description: "Requires explicit visibility declarations for class members.",
-        rationale: "Explicit visibility declarations can make code more readable and accessible for those new to TS.",
+        rationale: Lint.Utils.dedent`
+            Explicit visibility declarations can make code more readable and accessible for those new to TS.
+
+            Other languages such as C# default to \`private\`, unlike TypeScript's default of \`public\`.
+            Members lacking a visibility declaration may be an indication of an accidental leak of class internals.
+        `,
         optionsDescription: Lint.Utils.dedent`
             These arguments may be optionally provided:
 

src/rules/memberOrderingRule.ts

@@ -132,7 +132,13 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "member-ordering",
         description: "Enforces member ordering.",
-        rationale: "A consistent ordering for class members can make classes easier to read, navigate, and edit.",
+        rationale: Lint.Utils.dedent`
+            A consistent ordering for class members can make classes easier to read, navigate, and edit.
+
+            A common opposite practice to \`member-ordering\` is to keep related groups of classes together.
+            Instead of creating clases with multiple separate groups, consider splitting class responsibilities
+            apart across multiple single-responsibility classes.
+        `,
         optionsDescription,
         options: {
             type: "object",

src/rules/noAnyRule.ts

@@ -25,7 +25,17 @@ export class Rule extends Lint.Rules.AbstractRule {
         ruleName: "no-any",
         description: "Disallows usages of `any` as a type declaration.",
         hasFix: false,
-        rationale: "Using `any` as a type declaration nullifies the compile-time benefits of the type system.",
+        rationale: Lint.Utils.dedent`
+            Using \`any\` as a type declaration nullifies the compile-time benefits of the type system.
+
+            If you're dealing with data of unknown or "any" types, you shouldn't be accessing members of it.
+            Either add type annotations for properties that may exist or change the data type to the empty object type \`{}\`.
+
+            Alternately, if you're creating storage or handling for consistent but unknown types, such as in data structures
+            or serialization, use \`<T>\` template types for generic type handling.
+
+            Also see the \`no-unsafe-any\` rule.
+        `,
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],

src/rules/noBooleanLiteralCompareRule.ts

@@ -29,6 +29,10 @@ export class Rule extends Lint.Rules.TypedRule {
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],
+        rationale: Lint.Utils.dedent`
+            Comparing boolean values to boolean literals is unnecessary, as those expressions will result in booleans too.
+            Just use the boolean values directly or negate them.
+        `,
         type: "style",
         typescriptOnly: true,
         requiresTypeInfo: true,

src/rules/noConsecutiveBlankLinesRule.ts

@@ -28,7 +28,14 @@ export class Rule extends Lint.Rules.AbstractRule {
         ruleName: "no-consecutive-blank-lines",
         description: "Disallows one or more blank lines in a row.",
         hasFix: true,
-        rationale: "Helps maintain a readable style in your codebase.",
+        rationale: Lint.Utils.dedent`
+            Helps maintain a readable style in your codebase.
+
+            Extra blank lines take up extra space and add little to a semantic understanding of the code.
+            It can be harder to read through files when fewer components can fit into the screen.
+            If you find a file is so large you feel a need to split them up with extra blank lines or comments,
+            consider splitting your file into smaller files.
+        `,
         optionsDescription: Lint.Utils.dedent`
             An optional number of maximum allowed sequential blanks can be specified. If no value
             is provided, a default of ${Rule.DEFAULT_ALLOWED_BLANKS} will be used.`,

src/rules/noDynamicDeleteRule.ts

@@ -26,7 +26,14 @@ export class Rule extends Lint.Rules.AbstractRule {
         optionExamples: [true],
         options: null,
         optionsDescription: "Not configurable.",
-        rationale: "Deleting dynamically computed keys is dangerous and not well optimized.",
+        rationale: Lint.Utils.dedent`
+            Deleting dynamically computed keys is dangerous and not well optimized.
+
+            Also consider using a [\`Map\`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map)
+            or [\`Set\`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set)
+            if you're storing collections of objects.
+            Using \`Object\`s can cause occasional edge case bugs, such as if a key is named "hasOwnProperty".
+        `,
         ruleName: "no-dynamic-delete",
         type: "functionality",
         typescriptOnly: false,

src/rules/noFloatingPromisesRule.ts

@@ -25,7 +25,7 @@ export class Rule extends Lint.Rules.TypedRule {
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "no-floating-promises",
         description: "Promises returned by functions must be handled appropriately.",
-        descriptionDetails: "Use `no-unused-expression` in addition to this rule to reveal even more floating promises.",
+        descriptionDetails: "Unhandled Promises can cause unexpected behavior, such as resolving at unexpected times.",
         optionsDescription: Lint.Utils.dedent`
             A list of \'string\' names of any additional classes that should also be handled as Promises.
         `,
@@ -37,7 +37,14 @@ export class Rule extends Lint.Rules.TypedRule {
             },
         },
         optionExamples: [true, [true, "JQueryPromise"]],
-        rationale: "Unhandled Promises can cause unexpected behavior, such as resolving at unexpected times.",
+        rationale: Lint.Utils.dedent`
+            Creating a Promise and not storing or returning may lets other code run independent of of its results.
+            This can cause unexpected and/or non-deterministic behavior depending on external timing factors.
+
+            It's typically better to return Promises from functions that start them, then handle them in calling code.
+
+            Use \`no-unused-expression\` in addition to this rule to reveal even more floating promises.
+        `,
         type: "functionality",
         typescriptOnly: true,
         requiresTypeInfo: true,

src/rules/noInferredEmptyObjectTypeRule.ts

@@ -27,6 +27,11 @@ export class Rule extends Lint.Rules.TypedRule {
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],
+        rationale: Lint.Utils.dedent`
+            When function or constructor may be called with a type parameter but one isn't supplied or inferrable,
+            TypeScript defaults to \`{}\`.
+            This is often undesirable as the call is meant to be of a more specific type.
+        `,
         type: "functionality",
         typescriptOnly: true,
         requiresTypeInfo: true,

src/rules/noInvalidTemplateStringsRule.ts

@@ -28,6 +28,7 @@ export class Rule extends Lint.Rules.AbstractRule {
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],
+        rationale: "Interpolation will only work for template strings.",
         type: "functionality",
         typescriptOnly: false,
     };

src/rules/noMagicNumbersRule.ts

@@ -29,8 +29,9 @@ export class Rule extends Lint.Rules.AbstractRule {
             Disallows the use constant number values outside of variable assignments.
             When no list of allowed values is specified, -1, 0 and 1 are allowed by default.`,
         rationale: Lint.Utils.dedent`
-            Magic numbers should be avoided as they often lack documentation, forcing
-            them to be stored in variables gives them implicit documentation.`,
+            Magic numbers should be avoided as they often lack documentation.
+            Forcing them to be stored in variables gives them implicit documentation.
+        `,
         optionsDescription: "A list of allowed numbers.",
         options: {
             type: "array",

src/rules/noMisusedNewRule.ts

@@ -28,6 +28,11 @@ export class Rule extends Lint.Rules.AbstractRule {
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],
+        rationale: Lint.Utils.dedent`
+            Interfaces in TypeScript aren't meant to describe constructors on their implementations.
+            The \`new\` descriptor is primarily for describing JavaScript libraries.
+            If you're trying to describe a function known to be a class, it's typically better to \`declare class\`.
+        `,
         type: "functionality",
         typescriptOnly: true,
     };

src/rules/noNonNullAssertionRule.ts

@@ -24,7 +24,35 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "no-non-null-assertion",
         description: "Disallows non-null assertions using the `!` postfix operator.",
-        rationale: "Using non-null assertion cancels the benefits of the strict null checking mode.",
+        rationale: Lint.Utils.dedent`
+            Using non-null assertion cancels the benefits of the strict null checking mode.
+
+            Instead of assuming objects exist:
+
+            \`\`\`
+            function foo(instance: MyClass | undefined) {
+                instance!.doWork();
+            }
+            \`\`\`
+
+            Either inform the strict type system that the object must exist:
+
+            \`\`\`
+            function foo(instance: MyClass) {
+                instance.doWork();
+            }
+            \`\`\`
+
+            Or verify that the instance exists, which will inform the type checker:
+
+            \`\`\`
+            function foo(instance: MyClass | undefined) {
+                if (instance !== undefined) {
+                    instance.doWork();
+                }
+            }
+            \`\`\`
+        `,
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],

src/rules/noNullKeywordRule.ts

@@ -29,7 +29,26 @@ export class Rule extends Lint.Rules.AbstractRule {
         description: "Disallows use of the `null` keyword literal.",
         rationale: Lint.Utils.dedent`
             Instead of having the dual concepts of \`null\` and\`undefined\` in a codebase,
-            this rule ensures that only \`undefined\` is used.`,
+            this rule ensures that only \`undefined\` is used.
+
+            JavaScript originally intended \`undefined\` to refer to a value that doesn't yet exist,
+            while \`null\` was meant to refer to a value that does exist but points to nothing.
+            That's confusing.
+            \`undefined\` is the default value when object members don't exist, and is the return value
+            for newer native collection APIs such as \`Map.get\` when collection values don't exist.
+
+            \`\`\`
+            const myObject = {};
+            myObject.doesNotExist; // undefined
+            \`\`\`
+
+            \`\`\`
+            const myMap = new Map<string, number>();
+            myMap.get("doesNotExist"); // undefined
+            \`\`\`
+
+            To remove confusion over the two similar values, it's better to stick with just \`undefined\`.
+        `,
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],

src/rules/noParameterPropertiesRule.ts

@@ -27,7 +27,11 @@ export class Rule extends Lint.Rules.AbstractRule {
         description: "Disallows parameter properties in class constructors.",
         rationale: Lint.Utils.dedent`
             Parameter properties can be confusing to those new to TS as they are less explicit
-            than other ways of declaring and initializing class members.`,
+            than other ways of declaring and initializing class members.
+
+            It can be cleaner to keep member variable declarations in one list directly only the class
+            (instead of mixed between direct class members and constructor parameter properties).
+        `,
         optionsDescription: "Not configurable.",
         options: null,
         optionExamples: [true],

src/rules/noShadowedVariableRule.ts

@@ -28,7 +28,16 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "no-shadowed-variable",
         description: "Disallows shadowing variable declarations.",
-        rationale: "Shadowing a variable masks access to it and obscures to what value an identifier actually refers.",
+        rationale: Lint.Utils.dedent`
+            Shadowing a variable masks access to it and obscures to what value an identifier actually refers.
+            For example, in the following code, it can be confusing why the filter is likely never true:
+
+            \`\`\`
+            const findNeighborsWithin = (instance: MyClass, instances: MyClass[]): MyClass[] => {
+                return instances.filter((instance) => instance.neighbors.includes(instance));
+            };
+            \`\`\`
+        `,
         optionsDescription: Lint.Utils.dedent`
             You can optionally pass an object to disable checking for certain kinds of declarations.
             Possible keys are \`"class"\`, \`"enum"\`, \`"function"\`, \`"import"\`, \`"interface"\`, \`"namespace"\`, \`"typeAlias"\`