Problem statement: Smelling Code
Have you ever had to fix an error in someone else's code - but you had to reformat the code to even understand it? Incorrectly placed spaces, non-standard line breaks, inconsistent style for curly braces, etc.
Much has been written about the importance of a consistent code style across a team. And in the world of JavaScript, this is even more important, as the language itself has far fewer constraints than compiler-based languages like Java or C#.
In the IT world, the term "smelling" has become established for source code that does not contain any syntactic breaks at first, but looks unreadable, or where errors/problems can occur during execution.
Wikipedia defines it as follows:
"In computer programming, a code smell is any characteristic in the source code of a program that possibly indicates a deeper problem."
The important thing about code smells is that they don't necessarily pose a problem. Instead, they are signs that something is wrong with the code or that it is no longer maintainable because nobody understands what it is doing anymore.
As the responsible architect or team lead, you want to ensure that the JavaScript code in ServiceNow always looks "nice" and doesn't smell and thus there is a minimum level of source code quality - even if many different developers have contributed to it.
How Linters can help
Fortunately, there is a class of tools called "Linters" that will help you to achieve your quality goals. But what is a Linter? The term comes from a tool originally called “lint” that analyzed C source code. The computer scientist Stephen C. Johnson developed this utility in 1978 when he worked at Bell Labs. Both the original lint tool and earlier similar utilities had the goal of analyzing source code to suggest compiler optimizations. Over time, the lint-like tools added many other types of checks and verifications. But Linters - as they are called - aren’t restricted to compiled languages. They are way more valuable for interpreted languages like JavaScript since there’s no compiler to detect errors during development time.
And what can a Linter do for you? What are the benefits you can gain from them?
- Fewer errors in production:
The use of Linters helps diagnose and fix technical problems within the code. As a result, fewer errors find their way into production. - Readable, maintainable and consistent code:
Linters can help teams achieve a more readable and consistent style by enforcing its rules. - Fewer discussions about code style and aesthetics during code reviews:
Code review discussions should not be riddled with endless discussions about stylistic preferences. Linters can get these issues out of the way. - Objective measurement of code quality:
Discussions about code quality are often very subjective. Linters provide an objective and measurable assessment of code quality. - Safer and better performing code:
Not all Linters analyze source code for performance and security, but some do. - Code quality education reaches more developers:
Linters can help developers - especially the most inexperienced - learn about code quality.
As you have just read, the original Linter tool analyzed code to provide optimizations for compilers, but over time more advanced and complete tools have been released. Nowadays, there are countless different Linters and in the JavaScript world, one of the most famous is ESLint.
ESLint configuration in ServiceNow
via System Property
ServiceNow is also using ESLint under the hood for its script type fields. Since the New York release, you can specify at system property glide.ui.syntax_editor.linter.eslint_config all required rules. In a baseline instance, the following rules are pre-configured as a JSON object:
{
"rules": {
"constructor-super": "warn",
"no-case-declarations": "warn",
"no-class-assign": "warn",
"no-compare-neg-zero": "warn",
"no-cond-assign": "warn",
"no-console": "warn",
"no-const-assign": "warn",
"no-constant-condition": "warn",
"no-control-regex": "warn",
"no-debugger": "warn",
"no-delete-var": "warn",
"no-dupe-args": "warn",
"no-dupe-class-members": "warn",
"no-dupe-keys": "warn",
"no-duplicate-case": "warn",
"no-empty-character-class": "warn",
"no-empty-pattern": "warn",
"no-empty": ["warn", { "allowEmptyCatch": true }],
"no-ex-assign": "warn",
"no-extra-boolean-cast": "warn",
"no-extra-semi": "warn",
"semi" : "warn",
"no-fallthrough": "warn",
"no-func-assign": "warn",
"no-global-assign": "warn",
"no-inner-declarations": "warn",
"no-invalid-regexp": "warn",
"no-irregular-whitespace": "warn",
"no-mixed-spaces-and-tabs": "warn",
"no-new-symbol": "warn",
"no-obj-calls": "warn",
"no-octal": "warn",
"no-redeclare": "warn",
"no-regex-spaces": "warn",
"no-self-assign": "warn",
"no-sparse-arrays": "warn",
"no-this-before-super": "warn",
"no-undef": "off",
"no-unexpected-multiline": "warn",
"no-unreachable": "warn",
"no-unsafe-finally": "warn",
"no-unsafe-negation": "warn",
"no-unused-labels": "warn",
"no-unused-vars": "off",
"no-useless-escape": "warn",
"require-yield": "warn",
"use-isnan": "warn",
"valid-typeof": "warn"
}
}
As you can see in the above JSON structure there is a property "rules" which includes a list of rule names like "no-unreachable" along with instructions on how to handle the related findings. With "warn" the respective findings are underlined yellow and with "error" they are underlined red:
And you also can disable checks with the "off" option.
ℹ️ Since the Tokyo release there is another system property glide.ui.syntax_editor.linter.eslint_config.ecmascript2021 which seems to configure ESLint for the new ECMAScrip 2021 (ES12/ECMA6+) JavaScript engine introduced in that release. However, I could not find any documentation on how exactly this configuration is applied. Will the existing ESLint configuration be completely replaced, or will both be merged so that you only have to list the extensions in the second system property?
Via Code Comments
Another option to override the ESLint configurations from the system property is defining rules within the code with the help of comments:
Restrictions
Unfortunately, there are also a number of drawbacks and shortcomings that need to be mentioned:
- While real JavaScript syntax errors, such as a forgotten closing curly bracket, prevent a record from being saved, there is no (with known) way to enforce this for findings from the ESLint checks as well. If the developer decides to ignore the errors and warnings, it cannot be prevented with OOTB means.
- The configuration provided by the system property glide.ui.syntax_editor.linter.eslint_config applies to the whole instance. There is no way to differentiate between individual tables/fields or even script types (client-side vs. server-side).
- It is not clear which ESLint version ServiceNow is using. If you want to be sure that a certain rule is applied as documented or not, the only way is intensive testing.
- There is no easy way to find out the respective rule based on the error/warn message you get in the script editor. But the rule is important to understand the issue and to see examples on how to do it right. The best I could find was the following website, which maps error messages to the most famous JavaScript linters: http://linterrors.com/js
- In case the configuration does not represent a valid JSON, this is not reported anywhere with an error message - neither in the syslog nor in the browser console. For this reason, I recommend implementing a Business Rules which validates the ESLint configuration. Or you validate the configuration in any of the online available JSON testing and formatting tools like https://jsonformatter.curiousconcept.com/
- For many rules, automatic fixing would be possible, but I don't know any option in the script editor how to trigger that.
- In a typical ESLint configuration you wouldn't specify all the rules one by one but build upon best practices and modify only specific rules if required. In the configuration you could achieve that, for example, by using "extends":"eslint:recommended". However, this unfortunately did not work in my tests. For this reason, I have chosen the all-around approach and explicitly specified each and every rule that seemed to make sense to me.
- This leads to another problem. OOTB the "value" field at table sys_properties only allows 4000 characters and a complete ESLint configuration exceeds this limit. Strangely, the complete configuration is nevertheless stored in the database without being truncated. But to be on the safe side, you could extend the character limit in the table configuration.
My individual ESLint configurations
The following rule set is currently used by me in my projects. You can take it and customize it to meet your needs. The rules in this configuration partly take into account the special circumstances we have in ServiceNow and also represent my personal ideas of good-looking JavaScript code.
Please note the "globals" section at the end where I defined some global variable names we have in ServiceNow along with the most important OOTB classes/objects. That way, many false positive findings can be suppressed. But I recommend not to insert in that section project-specific or custom globals like Script Includes. Instead, use the mentioned inline comment options.
{
"rules": {
"accessor-pairs": "error",
"array-bracket-newline": ["error", "consistent"],
"array-bracket-spacing": ["error", "never"],
"array-callback-return": "error",
"array-element-newline": ["error", "consistent"],
"arrow-body-style": "error",
"arrow-parens": "error",
"arrow-spacing": "error",
"block-scoped-var": "error",
"block-spacing": "error",
"brace-style": ["error", "stroustrup"],
"capitalized-comments": "off",
"class-methods-use-this": "error",
"comma-dangle": ["error", "never"],
"comma-spacing": ["error", {"before": false, "after": true }],
"comma-style": ["error", "last"],
"complexity": ["error", 100],
"computed-property-spacing": ["error", "never"],
"consistent-return": "error",
"consistent-this": "off",
"constructor-super": "warn",
"curly": "error",
"default-case": "error",
"default-case-last": "error",
"dot-location": ["error", "property"],
"dot-notation": "off",
"eol-last": "off",
"eqeqeq": ["error", "always"],
"for-direction": "error",
"func-call-spacing": ["error", "never"],
"func-name-matching": "error",
"func-names": "off",
"func-style": "off",
"function-call-argument-newline": "off",
"function-paren-newline": "off",
"generator-star-spacing": "error",
"global-require": "error",
"guard-for-in": "error",
"handle-callback-err": "error",
"id-blacklist": "error",
"id-length": "off",
"id-match": "error",
"implicit-arrow-linebreak": "error",
"indent": "off",
"indent-legacy": "off",
"init-declarations": "off",
"jsx-quotes": "error",
"key-spacing": "off",
"keyword-spacing": "error",
"line-comment-position": "error",
"linebreak-style": "off",
"lines-around-comment": "off",
"lines-around-directive": "error",
"lines-between-class-members": "error",
"max-classes-per-file": "error",
"max-depth": ["error", 10],
"max-len": ["error", {"code": 115}],
"max-lines": "off",
"max-lines-per-function": "off",
"max-nested-callbacks": "error",
"max-params": ["error", 5],
"max-statements": "off",
"max-statements-per-line": "error",
"multiline-comment-style": "off",
"multiline-ternary": "off",
"new-cap": "error",
"new-parens": "error",
"newline-after-var": ["error", "always"],
"newline-before-return": "off",
"newline-per-chained-call": "error",
"no-alert": "error",
"no-array-constructor": "error",
"no-async-promise-executor": "error",
"no-await-in-loop": "error",
"no-bitwise": "error",
"no-buffer-constructor": "error",
"no-caller": "error",
"no-catch-shadow": "error",
"no-case-declarations": "error",
"no-class-assign": "error",
"no-compare-neg-zero": "error",
"no-cond-assign": "error",
"no-confusing-arrow": "error",
"no-console": "warn",
"no-const-assign": "error",
"no-constant-condition": ["error", {"checkLoops": false}],
"no-continue": "error",
"no-control-regex": "warn",
"no-debugger": "warn",
"no-delete-var": "error",
"no-div-regex": "error",
"no-duplicate-case": "error",
"no-duplicate-imports": "error",
"no-dupe-args": "error",
"no-dupe-class-members": "error",
"no-dupe-else-if": "error",
"no-dupe-keys": "error",
"no-else-return": "off",
"no-empty": "error",
"no-empty-character-class": "warn",
"no-empty-function": "error",
"no-empty-pattern": "warn",
"no-eq-null": "off",
"no-eval": "error",
"no-ex-assign": "error",
"no-extend-native": "error",
"no-extra-bind": "error",
"no-extra-boolean-cast": "warn",
"no-extra-label": "error",
"no-extra-parens": "off",
"no-extra-semi": "warn",
"no-fallthrough": "error",
"no-floating-decimal": "error",
"no-func-assign": "error",
"no-global-assign": "error",
"no-implicit-coercion": "off",
"no-implicit-globals": "off",
"no-implied-eval": "error",
"no-inline-comments": "error",
"no-inner-declarations": ["error", "functions"],
"no-invalid-this": "error",
"no-invalid-regexp": "warn",
"no-irregular-whitespace": "warn",
"no-iterator": "error",
"no-label-var": "error",
"no-labels": "error",
"no-lone-blocks": "error",
"no-lonely-if": "error",
"no-loop-func": "error",
"no-magic-numbers": "off",
"no-misleading-character-class": "error",
"no-mixed-operators": "error",
"no-mixed-requires": "error",
"no-mixed-spaces-and-tabs": "warn",
"no-multi-assign": "error",
"no-multi-spaces": "off",
"no-multi-str": "error",
"no-multiple-empty-lines": "error",
"no-new-symbol": "warn",
"no-native-reassign": "error",
"no-negated-condition": "off",
"no-negated-in-lhs": "error",
"no-nested-ternary": "error",
"no-new": "error",
"no-new-func": "error",
"no-new-object": "error",
"no-new-require": "error",
"no-new-wrappers": "error",
"no-obj-calls": "error",
"no-octal": "warn",
"no-octal-escape": "error",
"no-param-reassign": "off",
"no-path-concat": "error",
"no-plusplus": "off",
"no-process-env": "error",
"no-process-exit": "error",
"no-proto": "error",
"no-prototype-builtins": "error",
"no-redeclare": "warn",
"no-regex-spaces": "error",
"no-restricted-globals": "error",
"no-restricted-imports": "error",
"no-restricted-modules": "error",
"no-restricted-properties": "error",
"no-restricted-syntax": "error",
"no-return-assign": "error",
"no-return-await": "error",
"no-script-url": "error",
"no-self-assign": "error",
"no-self-compare": "error",
"no-sequences": "error",
"no-shadow": "error",
"no-shadow-restricted-names": "error",
"no-sparse-arrays": "warn",
"no-spaced-func": "error",
"no-sync": "error",
"no-tabs": "off",
"no-template-curly-in-string": "error",
"no-ternary": "off",
"no-this-before-super": "warn",
"no-throw-literal": "error",
"no-trailing-spaces": "off",
"no-undef": "error",
"no-undef-init": "error",
"no-undefined": "error",
"no-underscore-dangle": "off",
"no-unexpected-multiline": "error",
"no-unmodified-loop-condition": "error",
"no-unneeded-ternary": "error",
"no-unreachable": "error",
"no-unreachable-loop": "error",
"no-unsafe-finally": "warn",
"no-unsafe-negation": "warn",
"no-unused-expressions": "error",
"no-unused-labels": "warn",
"no-unused-vars": ["error", {"vars": "local", "args": "after-used", "ignoreRestSiblings": false}],
"no-use-before-define": "error",
"no-useless-call": "error",
"no-useless-catch": "error",
"no-useless-escape": "warn",
"no-useless-computed-key": "error",
"no-useless-concat": "error",
"no-useless-constructor": "error",
"no-useless-rename": "error",
"no-useless-return": "error",
"no-var": "off",
"no-void": "error",
"no-warning-comments": "error",
"no-whitespace-before-property": "error",
"no-with": "error",
"nonblock-statement-body-position": "error",
"object-curly-newline": "error",
"object-curly-spacing": ["error", "never"],
"object-property-newline": "warn",
"object-shorthand": "off",
"one-var": "off",
"one-var-declaration-per-line": "error",
"operator-assignment": "off",
"operator-linebreak": ["error", "after"],
"padded-blocks": "off",
"padding-line-between-statements": "error",
"prefer-arrow-callback": "off",
"prefer-const": "error",
"prefer-destructuring": "error",
"prefer-numeric-literals": "error",
"prefer-object-spread": "error",
"prefer-promise-reject-errors": "error",
"prefer-reflect": "off",
"prefer-rest-params": "off",
"prefer-spread": "error",
"prefer-template": "off",
"quote-props": "off",
"quotes": "off",
"radix": "error",
"require-atomic-updates": "error",
"require-await": "error",
"require-jsdoc": "off",
"require-unicode-regexp": "off",
"require-yield": "warn",
"rest-spread-spacing": "error",
"semi": "warn",
"semi-spacing": ["error", {"after": true, "before": false}],
"semi-style": ["error", "last"],
"sort-imports": "error",
"sort-keys": "off",
"sort-vars": "error",
"space-before-blocks": "off",
"space-before-function-paren": "always",
"space-in-parens": ["error", "never"],
"space-infix-ops": "error",
"space-unary-ops": "error",
"spaced-comment": "off",
"strict": ["error", "never"],
"switch-colon-spacing": "error",
"symbol-description": "error",
"template-curly-spacing": "error",
"template-tag-spacing": "error",
"unicode-bom": ["error", "never"],
"use-isnan": "error",
"valid-jsdoc": "off",
"vars-on-top": "off",
"wrap-iife": "off",
"wrap-regex": "off",
"yield-star-spacing": "error",
"yoda": ["error", "never"],
"valid-typeof": "warn"
},
"globals": {
"current": "writeable",
"engine": "readonly",
"global": "readonly",
"g_form": "readonly",
"g_user": "readonly",
"g_navigation": "readonly",
"g_list": "readonly",
"g_scratchpad": "readonly",
"gs": "readonly",
"previous": "readonly",
"request": "readonly",
"response": "readonly",
"sn_ws": "readonly",
"sn_ws_err": "readonly",
"sn_currency": "readonly",
"ArrayUtil": "readonly",
"Class": "readonly",
"CMDBUtil": "readonly",
"GlideAggregate": "writeable",
"GlideController": "readonly",
"GlideDateTime": "writeable",
"GlideDate": "writeable",
"GlideElement": "writeable",
"GlideEncrypter": "writeable",
"GlideEventManager": "writeable",
"GlideHTTPRequest": "writeable",
"GlideHTTPResponse": "readonly",
"GlideLocale": "readonly",
"GlideRecord": "writeable",
"GlideRecordSecure": "writeable",
"GlideRecordUtil": "readonly",
"GlideScopedEvaluator": "readonly",
"GlideSession": "readonly",
"GlideStringUtil": "readonly",
"GlideSysAttachment": "readonly",
"GlideUpdateManager2": "readonly",
"GlideUpdateSet": "readonly",
"j2js": "readonly",
"JSUtil": "readonly",
"TableUtils": "readonly"
}
}
