Functions code style linting (#18)

* code style: if

* Function code style tracking

* Type annotations linting

README.md

@@ -4,7 +4,6 @@
 [![Coverage Status](https://coveralls.io/repos/github/rokucommunity/bslint/badge.svg?branch=master)](https://coveralls.io/github/rokucommunity/bslint?branch=master)
 [![NPM Version](https://img.shields.io/npm/v/@rokucommunity/bslint.svg)](https://npmjs.org/package/bslint)
 
-
 [brighterscript](https://github.com/rokucommunity/brighterscript) is a Roku
 BrightScript compiler featuring many diagnostics out of the box: syntax check,
 function calls validation, script imports verification...
@@ -14,9 +13,6 @@ function calls validation, script imports verification...
 - a CLI tool to lint your code without compiling your project,
 - a `brighterscript` plugin offering **additional insights** on your code.
 
-
-
-
 ## Installation
 
 You need Node 10+ and `npm` or `yarn`:
@@ -70,11 +66,25 @@ and call `npm run lint`.
 
 Linting rules can be set in a `bslint.json` file in the root of your project.
 
+Rules are organised in 3 categories:
+
+- "Code style": how the code should look like for consistency
+- "Strictness": requirement to ensure code safety
+- "Code flow": tracks the code flow to identify risky patterns
+
 Default rules:
 
 ```json
 {
     "rules": {
+        "inline-if-style": "then",
+        "block-if-style": "no-then",
+        "condition-style": "no-group",
+        "named-function-style": "auto",
+        "anon-function-style": "auto",
+
+        "type-annotations": "off",
+
         "assign-all-paths": "error",
         "unsafe-path-loop": "error",
         "unsafe-iterators": "error",
@@ -86,38 +96,86 @@ Default rules:
 }
 ```
 
-Valid values for the rules severity are: `error | warn | info | off`.
+### Code style rules
+
+- `inline-if-style`: validation of inline `if/then` statements.
+
+    - `never`: do not allow,
+    - `no-then`: do not use `then` keyword
+    - `then`: always use `then` keyword (**default**)
+    - `off`: do not validate
+
+- `block-if-style`: validation of regular block `if/then` statements.
+
+    - `no-then`: do not use `then` keyword (**default**)
+    - `then`: always use `then` keyword
+    - `off`: do not validate
+
+- `condition-style`: validation of `if` statements conditions:
+  should the condition be wrapped around parenthesis?
+
+    - `no-group`: do not wrap with parenthesis (**default**)
+    - `then`: always wrap with parentheses
+    - `off`: do not validate
+
+- `named-function-style`, `anon-function-style`: validation of function style (`function/sub`)
 
-Rules:
+    - `no-function`: always use `sub`
+    - `no-sub`: always use `function`
+    - `auto`: use `sub` for `Void` functions, otherwise use `function` (**default**)
+    - `off`: no not validate
+
+### Strictness rules
+
+- `type-annotations`: validation of presence of `as` type annotations, for function arguments and return values.
+
+    - `all`: enforce both arguments and return type annotations
+    - `return`: enforce return type annotations
+    - `args`: engorce arguments type annotations
+    - `off`: do not validate (**default**)
+
+### Code flow rules
+
+Valid values for the rules severity are: `error | warn | info | off`.
 
 - `assign-all-paths`: a variable is not assigned in all the possible code paths,
-    ```brightscript
+
+    ```vb
     if a then
         b = "something"
     end if
     print b ' error
     ```
+
 - `unsafe-path-loop`: loops are considered as unsafe code paths: assignment in a
+
   loop may not happen.
-    ```brightscript
+
+    ```vb
     for i = 0 to n
         b = "something"
     end if
     print b ' b may not have been assigned
     ```
+
 - `unsafe-iterators`: loop iterator variable should not be used outside a loop
-    ```brightscript
+
+    ```vb
     for i = 0 to n
         b = "something"
     end if
     print i ' value could be invalid
     ```
+
 - `case-sensitivity`: inform of inconsistent variable casing
+
 - `unused-variable`: inform of variable being set but never used
+
 - `unreachable-code`: inform of unreachable code
-    ```brightscript
+    ```vb
     return
     print "is unreachable"
     ```
+
 - `consistent-return`: verifies consistency of `sub`/`function` returned values
   (missing return, missing value, returned value while function is `as void`,...)

src/index.ts

@@ -1,9 +1,15 @@
 import { BsConfig, Program } from 'brighterscript';
 import { DiagnosticSeverity } from 'brighterscript/dist/astUtils';
 import Linter from './Linter';
+import CodeStyle from './plugins/codeStyle';
 import TrackCodeFlow from './plugins/trackCodeFlow';
 
 export type RuleSeverity = 'error' | 'warn' | 'info' | 'off';
+export type RuleInlineIf = 'never' | 'no-then' | 'then' | 'off';
+export type RuleBlockIf = 'no-then' | 'then' | 'off';
+export type RuleCondition = 'no-group' | 'group' | 'off';
+export type RuleFunction = 'no-function' | 'no-sub' | 'auto' | 'off';
+export type RuleTypeAnnotations = 'all' | 'return' | 'args' | 'off';
 
 export type BsLintConfig = Pick<BsConfig, 'project' | 'rootDir' | 'files' | 'cwd' | 'watch'> & {
     lintConfig?: string;
@@ -18,8 +24,12 @@ export type BsLintConfig = Pick<BsConfig, 'project' | 'rootDir' | 'files' | 'cwd
         // 'no-stop'?: RuleSeverity,
         // 'only-function'?: RuleSeverity,
         // 'only-sub'?: RuleSeverity,
-        // 'no-single-line-if'?: RuleSeverity,
-        // 'no-optional-then'?: RuleSeverity,
+        'inline-if-style'?: RuleInlineIf;
+        'block-if-style'?: RuleBlockIf;
+        'condition-style'?: RuleCondition;
+        'named-function-style'?: RuleFunction;
+        'anon-function-style'?: RuleFunction;
+        'type-annotations'?: RuleTypeAnnotations;
     };
 };
 
@@ -33,6 +43,12 @@ export interface BsLintRules {
     caseSensitivity: BsLintSeverity;
     unusedVariable: BsLintSeverity;
     consistentReturn: BsLintSeverity;
+    inlineIfStyle: RuleInlineIf;
+    blockIfStyle: RuleBlockIf;
+    conditionStyle: RuleCondition;
+    namedFunctionStyle: RuleFunction;
+    anonFunctionStyle: RuleFunction;
+    typeAnnotations: RuleTypeAnnotations;
 }
 
 export { Linter };
@@ -42,6 +58,9 @@ export default function factory() {
         afterProgramCreate: (program: Program) => {
             const trackCodeFlow = new TrackCodeFlow(program);
             program.plugins.add(trackCodeFlow);
+
+            const codeStyle = new CodeStyle(program);
+            program.plugins.add(codeStyle);
         }
     };
 }

src/plugins/codeStyle/diagnosticMessages.ts

@@ -0,0 +1,83 @@
+import { DiagnosticSeverity, Range } from 'brighterscript';
+
+enum CodeStyleError {
+    InlineIfFound = 'LINT3001',
+    InlineIfThenMissing = 'LINT3002',
+    InlineIfThenFound = 'LINT3003',
+    BlockIfThenMissing = 'LINT3004',
+    BlockIfThenFound = 'LINT3005',
+    ConditionGroupMissing = 'LINT3006',
+    ConditionGroupFound = 'LINT3007',
+    SubKeywordExpected = 'LINT3008',
+    FunctionKeywordExpected = 'LINT3009',
+    ReturnTypeAnnotation = 'LINT3010',
+    TypeAnnotation = 'LINT3011',
+}
+
+const CS = 'Code style:';
+const ST = 'Strictness:';
+
+const messages = {
+    addBlockIfThenKeyword: (range: Range) => ({
+        severity: DiagnosticSeverity.Error,
+        code: CodeStyleError.BlockIfThenMissing,
+        message: `${CS} add 'then' keyword`,
+        range
+    }),
+    removeBlockIfThenKeyword: (range: Range) => ({
+        severity: DiagnosticSeverity.Error,
+        code: CodeStyleError.BlockIfThenFound,
+        message: `${CS} remove 'then' keyword`,
+        range
+    }),
+    inlineIfNotAllowed: (range: Range) => ({
+        severity: DiagnosticSeverity.Error,
+        code: CodeStyleError.InlineIfFound,
+        message: `${CS} no inline if statement allowed`,
+        range
+    }),
+    addInlineIfThenKeyword: (range: Range) => ({
+        severity: DiagnosticSeverity.Error,
+        code: CodeStyleError.InlineIfThenMissing,
+        message: `${CS} add 'then' keyword`,
+        range
+    }),
+    removeInlineIfThenKeyword: (range: Range) => ({
+        severity: DiagnosticSeverity.Error,
+        code: CodeStyleError.InlineIfThenFound,
+        message: `${CS} remove 'then' keyword`,
+        range
+    }),
+    addParenthesisAroundCondition: (range: Range) => ({
+        severity: DiagnosticSeverity.Error,
+        code: CodeStyleError.ConditionGroupMissing,
+        message: `${CS} add parenthesis around condition`,
+        range
+    }),
+    removeParenthesisAroundCondition: (range: Range) => ({
+        severity: DiagnosticSeverity.Error,
+        code: CodeStyleError.ConditionGroupFound,
+        message: `${CS} remove parenthesis around condition`,
+        range
+    }),
+    expectedKeyword: (range: Range, keyword: string, reason: string) => ({
+        severity: DiagnosticSeverity.Error,
+        code: CodeStyleError.SubKeywordExpected,
+        message: `${CS} expected '${keyword}' keyword ${reason}`,
+        range
+    }),
+    expectedReturnTypeAnnotation: (range: Range) => ({
+        severity: DiagnosticSeverity.Error,
+        code: CodeStyleError.ReturnTypeAnnotation,
+        message: `${ST} function should declare the return type`,
+        range
+    }),
+    expectedTypeAnnotation: (range: Range) => ({
+        severity: DiagnosticSeverity.Error,
+        code: CodeStyleError.TypeAnnotation,
+        message: `${ST} type annotation required`,
+        range
+    })
+};
+
+export default messages;