self review comments/tweaks

Author: zachbryant

docs/rules/enum.md

@@ -18,16 +18,17 @@ enum U {
 }
 
 // Case-sensitive by default.
-enum U {
+enum V {
   a = 'T',
-  b = 'T',
-  C = 'T',
+  B = 'T',
+  c = 'T',
 }
 
-enum U {
+enum W {
   a = 'T',
-  'c' = 'T',
+  'c+-' = 'T',
   b = 'T',
+  d = 'T',
 }
 ```
 
@@ -43,25 +44,30 @@ enum U {
 }
 
 // Case-sensitive by default.
-enum U {
-  C = 'T',
+enum V {
+  B = 'T',
   a = 'T',
-  b = 'T',
+  c = 'T',
 }
 
 // This rule sorts members which have an arbitrary string literal name as well.
-enum U {
+enum W {
   a = 'T',
-  'b+-' = 'T',
-  c = 'T',
+  b = 'T',
+  'c+-' = 'T',
+  d = 'T',
 }
 ```
 
 ## Options
 
 ```json
 {
-  "typescript-sort-keys/enum": ["error", "asc", { "caseSensitive": true, "natural": false }]
+  "typescript-sort-keys/enum": [
+    "error",
+    "asc",
+    { "caseSensitive": true, "natural": false }
+  ]
 }
 ```
 
@@ -82,19 +88,14 @@ Examples of **incorrect** code for the `"desc"` option:
 ```ts
 /* eslint typescript-sort-keys/enum: ["error", "desc"] */
 
-enum U {
-  b = 'T',
-  c = 'T',
-  a = 'T',
-}
 enum U {
   b = 'T',
   c = 'T',
   a = 'T',
 }
 
 // Case-sensitive by default.
-enum U {
+enum V {
   a = 'T',
   B = 'T',
   c = 'T',
@@ -106,19 +107,14 @@ Examples of **correct** code for the `"desc"` option:
 ```ts
 /* eslint typescript-sort-keys/enum: ["error", "desc"] */
 
-enum U {
-  c = 'T',
-  b = 'T',
-  a = 'T',
-}
 enum U {
   c = 'T',
   b = 'T',
   a = 'T',
 }
 
 // Case-sensitive by default.
-enum U {
+enum V {
   c = 'T',
   a = 'T',
   B = 'T',
@@ -138,7 +134,7 @@ enum U {
   C = 'T',
   b = 'T',
 }
-enum U {
+enum V {
   a = 'T',
   C = 'T',
   c = 'T',
@@ -157,7 +153,7 @@ enum U {
   c = 'T',
   C = 'T',
 }
-enum U {
+enum V {
   a = 'T',
   b = 'T',
   C = 'T',

docs/rules/string-enum.md

@@ -2,13 +2,13 @@
 
 ---
 
-# require string-enum members to be sorted (string-enum)
+# require string enum members to be sorted (string-enum)
 
-When declaring multiple members on a string-enum, some developers prefer to sort string-enum member names alphabetically to be able to find necessary members easier at the later time. Others feel that it adds complexity and becomes burden to maintain.
+When declaring multiple members on a string enum, some developers prefer to sort string enum member names alphabetically to be able to find necessary members easier at the later time. Others feel that it adds complexity and becomes burden to maintain.
 
 ## Rule Details
 
-This rule checks all members of a string-enum declaration and verifies that all keys are sorted alphabetically.
+This rule checks all members of a string enum declaration and verifies that all keys are sorted alphabetically.
 
 Examples of **incorrect** code for this rule:
 
@@ -81,13 +81,13 @@ enum U {
 
 The 1st option is `"asc"` or `"desc"`.
 
-- `"asc"` (default) - enforce string-enum members to be in ascending order.
-- `"desc"` - enforce string-enum members to be in descending order.
+- `"asc"` (default) - enforce string enum members to be in ascending order.
+- `"desc"` - enforce string enum members to be in descending order.
 
 The 2nd option is an object which has 2 properties.
 
-- `caseSensitive` - if `true`, enforce string-enum members to be in case-sensitive order. Default is `true`.
-- `natural` - if `true`, enforce string-enum members to be in natural order. Default is `false`. Natural Order compares strings containing combination of letters and numbers in the way a human being would sort. It basically sorts numerically, instead of sorting alphabetically. So the number 10 comes after the number 3 in Natural Sorting.
+- `caseSensitive` - if `true`, enforce string enum members to be in case-sensitive order. Default is `true`.
+- `natural` - if `true`, enforce string enum members to be in natural order. Default is `false`. Natural Order compares strings containing combination of letters and numbers in the way a human being would sort. It basically sorts numerically, instead of sorting alphabetically. So the number 10 comes after the number 3 in Natural Sorting.
 
 ### desc
 
@@ -209,4 +209,4 @@ enum U {
 
 ## When Not To Use It
 
-If you don't want to notify about string-enum members' order, then it's safe to disable this rule.
+If you don't want to notify about string enum members' order, then it's safe to disable this rule.

src/common/options.ts

@@ -1,12 +1,7 @@
-import {
-  AllRuleOptions,
-  CreateReporterArgs,
-  SortingOrder,
-  SortingParamsOptions,
-} from '../types'
+import { AllRuleOptions, CreateReporterArgs, SortingOrder, SortingParams } from '../types'
 
 export const defaultSortingOrder = SortingOrder.Ascending
-export const defaultOptions: SortingParamsOptions = {
+export const defaultOptions: SortingParams = {
   caseSensitive: true,
   natural: false,
   requiredFirst: false,

src/fixer.ts

@@ -12,7 +12,7 @@ export const getFixerFunction = (
   sortedBody: TSType[],
 ): ReportFixFunction =>
   function* (fixer: TSESLint.RuleFixer) {
-    const sourceCode = createReporterArgs.context.getSourceCode() as SourceCode
+    const sourceCode = createReporterArgs.context.sourceCode as SourceCode
 
     const bodyRange = memoize(`bodyRange_${baseMemoKey}`, () =>
       getBodyRange(sourceCode, body as unknown as Node[]),

src/plugin.ts

@@ -1,4 +1,5 @@
 import { TSESTree } from '@typescript-eslint/utils'
+
 import { getOptions } from './common/options'
 import { getFixerFunction } from './fixer'
 import { reportBodyNodes, reportParentNode } from './report'
@@ -47,7 +48,7 @@ export function createReporter(
       return
     }
 
-    const sourceCode = createReporterArgs.context.getSourceCode()
+    const sourceCode = createReporterArgs.context.sourceCode
     // Create a key for memoizing results based on plugin context & input
     const baseMemoKey = JSON.stringify({
       body: sourceCode.getText(bodyParent), // Disambiguator when body has code embedded in body

src/rules/enum.ts

@@ -7,7 +7,7 @@ import {
   RuleOptionsGeneric,
   SortingOrder,
   sortingOrderOptionSchema,
-  SortingParamsOptions,
+  SortingParams,
 } from '../types'
 import { getObjectBody } from '../utils/ast'
 import { createRule, RuleMetaData } from '../utils/rule'
@@ -16,12 +16,11 @@ import { createRule, RuleMetaData } from '../utils/rule'
  * The name of this rule.
  */
 export const name = 'enum' as const
-export const nameDeprecated = 'string-enum' as const
 
 /**
  * The options this rule can take.
  */
-export type RuleOptions = RuleOptionsGeneric<Omit<SortingParamsOptions, 'requiredFirst'>>
+export type RuleOptions = RuleOptionsGeneric<Omit<SortingParams, 'requiredFirst'>>
 
 const sortingParamsOptionSchema: JSONSchema4 = {
   type: 'object',
@@ -62,7 +61,7 @@ type errorMessageKeys = keyof typeof errorMessages
  * The meta data for this rule.
  */
 const meta: RuleMetaData<errorMessageKeys> = {
-  type: 'layout',
+  type: 'suggestion',
   docs: {
     description: 'require enum members to be sorted',
     recommended: 'stylistic',

src/rules/interface.ts

@@ -7,7 +7,7 @@ import {
   RuleOptionsGeneric,
   SortingOrder,
   sortingOrderOptionSchema,
-  SortingParamsOptions,
+  SortingParams,
 } from '../types'
 import { getObjectBody } from '../utils/ast'
 import { createRule, RuleMetaData } from '../utils/rule'
@@ -20,7 +20,7 @@ export const name = 'interface' as const
 /**
  * The options this rule can take.
  */
-export type RuleOptions = RuleOptionsGeneric<SortingParamsOptions>
+export type RuleOptions = RuleOptionsGeneric<SortingParams>
 
 const sortingParamsOptionSchema: JSONSchema4 = {
   type: 'object',
@@ -67,7 +67,7 @@ type errorMessageKeys = keyof typeof errorMessages
  * The meta data for this rule.
  */
 const meta: RuleMetaData<errorMessageKeys> = {
-  type: 'layout',
+  type: 'suggestion',
   docs: {
     description: 'require interface keys to be sorted',
     recommended: 'stylistic',

src/rules/string-enum.ts

@@ -7,7 +7,7 @@ import {
   RuleOptionsGeneric,
   SortingOrder,
   sortingOrderOptionSchema,
-  SortingParamsOptions,
+  SortingParams,
 } from '../types'
 import { getObjectBody } from '../utils/ast'
 import { createRule, RuleMetaData } from '../utils/rule'
@@ -22,7 +22,7 @@ export const name = 'string-enum' as const
  * @deprecated
  * The options this rule can take.
  */
-export type RuleOptions = RuleOptionsGeneric<Omit<SortingParamsOptions, 'requiredFirst'>>
+export type RuleOptions = RuleOptionsGeneric<Omit<SortingParams, 'requiredFirst'>>
 
 const sortingParamsOptionSchema: JSONSchema4 = {
   type: 'object',
@@ -66,7 +66,7 @@ type errorMessageKeys = keyof typeof errorMessages
  * The meta data for this rule.
  */
 const meta: RuleMetaData<errorMessageKeys> = {
-  type: 'layout',
+  type: 'suggestion',
   docs: {
     description: 'require string enum members to be sorted',
     recommended: 'stylistic',

src/types/options.ts

@@ -10,9 +10,7 @@ export const sortingOrderOptionSchema: JSONSchema4 = {
   type: 'string',
 }
 
-export type SortingOrderOption = SortingOrder
-
-export interface SortingParamsOptions {
+export interface SortingParams {
   readonly caseSensitive: boolean
   readonly natural: boolean
   readonly requiredFirst: boolean

src/types/rule.ts

@@ -1,8 +1,5 @@
-import { SortingOrderOption, SortingParamsOptions } from './options'
+import { SortingOrder, SortingParams } from './options'
 
-export type RuleOptionsGeneric<T> =
-  | []
-  | [SortingOrderOption]
-  | [SortingOrderOption, Partial<T>]
+export type RuleOptionsGeneric<T> = [] | [SortingOrder] | [SortingOrder, Partial<T>]
 
-export type AllRuleOptions = RuleOptionsGeneric<SortingParamsOptions>
+export type AllRuleOptions = RuleOptionsGeneric<SortingParams>

src/utils/reportUtils.ts

@@ -1,8 +1,6 @@
 import { PLUGIN_NAME } from 'config/constants'
-import {
-  name as enumRuleName,
-  nameDeprecated as enumRuleNameDeprecated,
-} from 'rules/enum'
+import { name as enumRuleName } from 'rules/enum'
+import { name as enumRuleNameDeprecated } from 'rules/string-enum'
 import { NodePositionInfo, TSType } from 'types'
 
 import { getRuleDocsUrl } from './rule'
@@ -52,7 +50,7 @@ export function getUnsortedInfo(
 export function getDeprecationMessage(name: string) {
   switch (name) {
     case enumRuleNameDeprecated:
-      return `DEPRECATED: see [${PLUGIN_NAME}/enum](${getRuleDocsUrl(enumRuleName)})`
+      return `DEPRECATED: see [${PLUGIN_NAME}/${enumRuleName}](${getRuleDocsUrl(enumRuleName)})`
     default:
       return ''
   }

src/utils/sourcecode/index.ts

@@ -1,5 +1,7 @@
-import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'
 import { Node, SourceCode } from 'types'
+
+import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'
+
 import {
   getCommentsTextAfter,
   getCommentsTextBefore,
@@ -11,6 +13,8 @@ import { getProcessedText } from './textHelpers'
 
 /**
  * Returns the text of the entire body, rebuilt from the source code in order given.
+ * Preserves location of comments in relation to members, and ensures line comments
+ * don't move and comment out other members.
  */
 export function getFixedBodyText(
   sourceCode: SourceCode,

tests/helpers/cases/index.ts

@@ -11,7 +11,8 @@ export { processInvalidTestCase, processValidTestCase } from './process'
 
 /**
  * Helper for splitting up test cases by options and displaying info about which
- * set of options the test is running with. Sacrifices some parallelizatiom for debuggability.
+ * set of options the test is running with. Sacrifices some parallelizatiom for
+ * powerful concise expression in test files.
  */
 export function runCases(
   {
@@ -24,11 +25,10 @@ export function runCases(
   runOptions: { withRequiredFirstOption: boolean; category: CaseCategory },
 ) {
   const usedOptionsKeys = new Set(Object.keys(invalid).concat(Object.keys(valid)))
+  const ruleTester = new RuleTester(typescriptConfig)
 
   for (const optionsSetKey of usedOptionsKeys) {
     describe(optionsSetKey, () => {
-      const ruleTester = new RuleTester(typescriptConfig)
-
       ruleTester.run(name, rule as unknown as Rule.RuleModule, {
         valid: processValidTestCase(
           { [optionsSetKey]: valid[optionsSetKey] },

tests/helpers/cases/preprocess.ts

@@ -40,6 +40,7 @@ function processErrorArgs(
     errorMessages.push(getCountErrorString(category, errorArgs.length))
 
   for (const args of errorArgs) {
+    // At this point args is number or string[]
     if (Array.isArray(args)) {
       switch (args.length) {
         case 1:
@@ -83,6 +84,7 @@ export function preProcessInvalidTestCases(
   return processedCases
 }
 
+// Take from shape PreInvalidTestCase[] to InvalidTestCase[]
 function convertPreInvalidCasesToProcessed(
   cases: PreInvalidTestCaseList,
   category: CaseCategory,
@@ -130,6 +132,7 @@ export function preProcessValidTestCases(
   return processedCases
 }
 
+// Take from shape PreValidTestCase[] to ValidTestCase[]
 function convertPreValidCasesToProcessed(
   cases: string[],
   withRequiredFirstOption: boolean,