How we use Teamscale's Custom Checks to Improve our Code

Posted on 10/25/2017 by Daniel Veihelmann

As you probably know, our quality tool Teamscale comes with a lot of checks for your programming language(s) of choice. These checks can, for example, help you to write better code by pointing out logical flaws, copy&paste programming and possible performance bottlenecks. But did you know that you can easily implement customized checks for your code base? In this post, I’ll show how we use this to improve our own code base by presenting two specific examples.

Custom Checks in Teamscale

In addition to the various checks that come »built-in« with Teamscale, it is really easy to add custom checks that are specific for your code base. My colleague Elijah has an excellent overview for you when it comes to implementing a new Teamscale check. In this post, I’ll describe two examples for checks that we recently implemented and that are tailored to our own code base.

Example 1: Use new language features, please

It is always exciting to see new features coming to the language you are working with. For example, there may now be more concise ways to describe a certain behavior in code, which leads to less code or reduces the risk of bugs. So even though using the language the »old« way is correct, you really would like to see the new language features to be embraced in your code base. How can you ensure that you and your colleagues are aware of this? One way is to add a custom check in Teamscale, which will create a finding whenever the new language feature can be used, especially in new or changed code.

Here is a simple but real-world example from our own JavaScript code base. Before the ECMAScript 2015 (ES6) JavaScript standard arrived, there was only one keyword to declare a variable, namely var. The ES6 standard brought two new alternatives, namely const (for immutable) variables and let (mutable). If you tried to change a variable that was declared with const, you would see a runtime error thrown by the browser, because this behavior is not allowed. In addition, these two new keywords make it easier to express your intent (»Will I change this variable somewhere in the current code?«).

Because of this, we now prefer the new const and let over the old var keyword. Let’s see how we can bake this into a Teamscale custom check. In this particular case, this is almost trivial to do as var happens to be a keyword and the check base class UnacceptedTokensCheckBase (which is itself very small) already gives us a skeleton for this check. Here is the final check that discourages usages of the var keyword:

 1@ACheck(name = "Use ES6 keywords for variable declarations (not 'var')", 
 2description = DoNotUseVarKeywordCheck.FINDING_DESCRIPTION, groupName = "Javascript (built-in)", 
 3defaultEnablement = EFindingEnablement.YELLOW, languages = {
 4                ELanguage.JAVASCRIPT }, parameters = { ECheckParameter.ABSTRACT_SYNTAX_TREE })
 5public class DoNotUseVarKeywordCheck extends UnacceptedTokensCheckBase {
 6
 7        protected final static String FINDING_DESCRIPTION = 
 8                "The var keyword in JavaScript should be replaced with const (for immutable variables) or let (mutable)";
 9
10        /** {@inheritDoc} */
11        @Override
12        protected EnumSet<ETokenType> getUnacceptedTokens() {
13                return EnumSet.of(ETokenType.VAR);
14        }
15
16        /** {@inheritDoc} */
17        @Override
18        protected String getFindingMessage(IToken findingToken) {
19                return "var keyword should be let or const instead";
20        }
21}

Pretty simple, right? Now, for the implementation of this particular check, it is obviously very helpful that the var happens to be language keyword. Let’s see how we can implement a check for something that is slightly more complex:

Example 2: A consistent way of doing things in code

When you are writing code, you may sometimes find yourself in a situation where you have to choose between two (or more) options as to how to achieve something. Here is a small example from the JavaScript world: If you have an JavaScript object, e.g., const myProduct = {name : 'Teamscale', version: '3.7.2'}, you can access it’s properties name and version in (at least) two ways: You can either use myProduct['name'] (»bracket notation«) or myProduct.name (»dot notation«), which will both yield »Teamscale«. (By the way: did you notice that we used the const keyword for declaring our myProduct variable? ;-) )

In the past, our JavaScript guidelines stated that we should prefer the bracket notation. However, as this has recently changed, we now want to continuously replace all the bracket notation occurrences with dot notation. Of course, it was only logical for us to add a corresponding Teamscale check for own codebase. Let’s see how this check is implemented:

 1@ACheck(name = "Bracket notation should be replaced by dot notation", 
 2description = "(Omitted here)", groupName = CheckGroupNames.BAD_PRACTICE, 
 3defaultEnablement = EFindingEnablement.OFF, languages = {
 4                ELanguage.JAVASCRIPT }, parameters = { ECheckParameter.ABSTRACT_SYNTAX_TREE })
 5public class DoNotUseBracketNotationCheck extends TokenPatternJavaScriptCheckBase {
 6
 7        /** {@inheritDoc} */
 8        @Override
 9        protected ETokenType[] getTokenPattern() {
10            // LBRACK is '[', RBRACK is ']'
11                return new ETokenType[] { IDENTIFIER, LBRACK, STRING_LITERAL, RBRACK };
12        }
13
14        /** {@inheritDoc} */
15        @Override
16        protected void processFoundSequence(UnmodifiableList<IToken> tokens, int indexOfFindingSequence)
17                        throws CheckException {
18
19                String accessedFieldName = tokens.get(indexOfFindingSequence + 2).getText();
20
21                // If the field string contains a space, it is probably a a false positive
22                // (e.g. a['some value'] could not be written with dot-notation)
23                if (!accessedFieldName.contains(SPACE)) {
24                        String simpleName = accessedFieldName.replaceAll("'", StringUtils.EMPTY_STRING);
25                        createFinding("Field access ['" + simpleName + "'] should be replaced with ." 
26                                                        + simpleName, tokens.get(indexOfFindingSequence));
27                }
28        }
29}

This check will check all JavaScript statements and look for the tokens we returned via the getTokenPattern() method. (A token is basically a single unit of information in code, e.g., a bracket or the name of an identifier.) Any matches will be passed to the processFoundSequence() method for further inspection. In our case, we only need to do a simple false-positive check. If a false positive seems unlikely, we then call createFinding() with a customized finding message.

Let’s see how this looks how an instance of this finding looks in Teamscale:

Our finding in Teamscale

Naturally, as we widely used the bracket notation in our codebase earlier, it will take some time to adapt (most of) our existing code to the new standard. However, since we now have Teamscale findings for these cases and we adhere to the boy-scout rule (»leave the camp ground cleaner than you found it«) in our own development at CQSE, we will see less and less occurrences of the outdated notation.

Custom checks for your code base

Can you think of specific examples in your code that should (not) be done but where simply adding a comment is not possible or sufficient? This might include performance anti-patterns, consistency checks or ignoring built-in language features. If your answer is »yes«, it might be a good idea to add a custom check in Teamscale. Of course, as many of our customers are already quite busy with their usual work load, my colleagues and I will be happy to help you in this process, simply drop us a line!

In any case, I hope that this blog post was interesting to you. Feel free to share your experiences, questions or other thoughts in the comment section.