Forem: Saif Sadiq

JavaScript best practices to improve code quality

Saif Sadiq — Mon, 19 Apr 2021 09:58:30 +0000

If you write JavaScript today, it’s worth your time staying in the know of all the updates the language has seen in the past few years. Since 2015, with the release of ES6, a new version of the ECMAScript spec has been released each year. Each iteration adds new features, new syntax, and Quality of Life improvements to the language. JavaScript engines in most browsers and Node.js quickly catch up, and it’s only fair that your code should catch up as well. That’s because with each new iteration of JavaScript comes new idioms and new ways to express your code, and many a time, these changes may make the code more maintainable for you and your collaborators.

Here are some of the latest ECMAScript features, and by induction, JavaScript and Node.js that you can make use of to write cleaner, more concise, and more readable code.

1. Block scored declarations

Since the inception of the language, JavaScript developers have used var to declare variables. The keyword var has its quirks, the most problematic of those being the scope of the variables created by using it.

var x = 10
if (true) { var x = 15 // inner declaration overrides declaration in parent scope
 console.log(x) // prints 15
}
console.log(x) // prints 15

Since variables defined with var are not block-scoped, redefining them in a narrower scope affects the value of the outer scope.

Now we have two new keywords that replace var, namely let and const that do not suffer from this drawback.

let y = 10
if (true) { let y = 15 // inner declaration is scoped within the if block
 console.log(y) // prints 15
}
console.log(y) // prints 10

const and let differ in the semantics that variables declared with const cannot be reassigned in their scope. This does not mean they are immutable, only that their references cannot be changed.

const x = [] x.push("Hello", "World!")
x // ["Hello", "World!"]

x = [] // TypeError: Attempted to assign to readonly property.

2. Arrow functions

Arrow functions are another very important feature introduced recently to JavaScript. They come bearing many advantages. First and foremost, they make the functional aspects of JavaScript beautiful to look at and simpler to write.

let x = [1, 2, 3, 4] x.map(val => val * 2) // [2, 4, 6, 8]
x.filter(val => val % 2 == 0) // [2, 4]
x.reduce((acc, val) => acc + val, 0) // 10

In all of the above examples the arrow functions, named after the distinctive arrow =>, replace traditional functions with a concise syntax.

If the function body is a single expression, the scope brackets {} and return keyword are implied and need not be written.
If the function has a single argument, the argument parentheses () are implied and need not be written.
If the function body expression is a dictionary, it must be enclosed in parentheses ().

Another significant advantage of arrow functions is that they do not define a scope but rather exist within the parent scope. This avoids a lot of pitfalls that can arise with the use of the this keyword. Arrow functions have no bindings for this. Inside the arrow function, the value of this is the same as that in the parent scope. Consequently, arrow functions cannot be used as methods or constructors. Arrow functions don’t work with apply, bind, or call and have no bindings for super.

They also have certain other limitations such as lack of the arguments object which traditional functions can access and the inability to yield from the function body.

Thus arrow functions are not a 1:1 replacement for standard functions but welcome addition to JavaScript’s feature set.

3. Optional chaining

Imagine a deeply nested data structure like this person object here. Consider you wanted to access the first and last name of this person. You would write this in JavaScript like so:

person = { name: { first: 'John', last: 'Doe', }, age: 42
}
person.name.first // 'John'
person.name.last // 'Doe'

Now imagine what would happen if the person object did not contain a nested name object.

person = { age: 42
}
person.name.first // TypeError: Cannot read property 'first' of undefined
person.name.last // TypeError: Cannot read property 'last' of undefined

To avoid such errors, developers had to resort to code like the following, which is unnecessarily verbose, hard to read, and unpleasant to write — a very bad trio of adjectives.

person && person.name && person.name.first // undefined

Meet optional chaining, a new feature of JavaScript that does away with this monstrosity. Optional chaining short-circuits the digging process as soon as it encounters a null or undefined value and returns undefined without raising an error.

person?.name?.first // undefined

The resultant code is much concise and cleaner.

4. Null-ish coalescing

Before introducing the null-ish coalescing operator, JavaScript developers used the OR operator || to fall back to a default value if the input was absent. This came with a significant caveat that even legitimate but falsy values would result in a fallback to the defaults.

function print(val) { return val || 'Missing'
} print(undefined) // 'Missing'
print(null) // 'Missing'

print(0) // 'Missing'
print('') // 'Missing'
print(false) // 'Missing'
print(NaN) // 'Missing'

JavaScript has now proposed the null coalescing operator ??, which offers a better alternative in that it only results in a fallback if the preceding expression is null-ish. Here null-ish refers to values that are null or undefined.

function print(val) { return val ?? 'Missing'
} print(undefined) // 'Missing'
print(null) // 'Missing'

print(0) // 0
print('') // ''
print(false) // false
print(NaN) // NaN

This way, you can ensure that if your program accepts falsy values as legitimate inputs, you won’t end up replacing them with fallbacks.

5. Logical assignment

Let’s say you want to assign a value to a variable if and only if the value is currently null-ish. A logical way to write this would be like so:

if (x === null || x == undefined) { x = y
}

If you knew about how short-circuiting works, you might want to replace those 3 lines of code with a more succinct version using the null-ish coalescing operator.

x ?? (x = y) // x = y if x is nullish, else no effect

Here we use the short-circuiting feature of the null-ish coalescing operator to execute the second part x = y if x is null-ish. The code is pretty concise, but it still is not very easy to read or understand. The logical null-ish assignment does away with the need for such a workaround.

x ??= y // x = y if x is nullish, else no effect

Along the same lines, JavaScript also introduces logical AND assignment &&= and logical OR assignment ||= operators. These operators perform assignment only when the specific condition is met and have no effect otherwise.

x ||= y // x = y if x is falsy, else no effect
x &&= y // x = y if x is truthy, else no effect

Pro-tip: If you’ve written Ruby before, you’ve seen the ||= and &&= operators, since Ruby does not have the concept of falsy values.

6. Named capture groups

Let’s start with a quick recap of capture groups in regular expressions. A capture group is a part of the string that matches a portion of regex in parentheses.

let re = /(\d{4})-(\d{2})-(\d{2})/
let result = re.exec('Pi day this year falls on 2021-03-14!') result[0] // '2020-03-14', the complete match
result[1] // '2020', the first capture group
result[2] // '03', the second capture group
result[3] // '14', the third capture group

Regular expressions have also supported named capture groups for quite some time, which is a way for the capture groups to be referenced by a name rather than an index. Now, with ES9, this feature has made its way to JavaScript. Now the result object contains a nested groups object where each capture group’s value is mapped to its name.

let re = /(?<year>\d{4})-(?<month>\d{2})-(?<day>\d{2})/
let result = re.exec('Pi day this year falls on 2021-03-14!') result.groups.year // '2020', the group named 'year'
result.groups.month // '03', the group named 'month'
result.groups.day // '14', the group named 'day'

The new API works beautifully with another new JavaScript feature, de-structured assignments.

let re = /(?<year>\d{4})-(?<month>\d{2})-(?<day>\d{2})/
let result = re.exec('Pi day this year falls on 2021-03-14!')
let { year, month, day } = result.groups year // '2020'
month // '03'
day // '14'

7. `async` & `await`

One of the powerful aspects of JavaScript is its asynchronicity. This means that many functions that may be long-running or time-consuming can return a Promise and not block execution.

const url = 'https://the-one-api.dev/v2/book'
let prom = fetch(url)
prom // Promise {<pending>}

// wait a bit
prom // Promise {<fullfilled>: Response}, if no errors
// or
prom // Promise {<rejected>: Error message}, if any error

Here the call to fetch returns a Promise that has the status ‘pending’ when created. Soon, when the API returns the response, it transitions into a ‘fulfilled’ state, and the Response that it wraps can be accessed. In the Promises world, you would do something like this to make an API call and parse the response as JSON.

const url = 'https://the-one-api.dev/v2/book'
let prom = fetch(url)
prom // Promise {<fullfilled>: Response}
 .then(res => res.json()) .then(json => console.log(json)) // prints response, if no errors
 .catch(err => console.log(err)) // prints error message, if any error

In 2017, JavaScript announced two new keywords async and await, that make handling and working with Promises easier and more fluent. They are not a replacement for Promises; they are merely syntactic sugar on top of the powerful Promises concepts.

Instead of all the code happening inside a series of ‘then’ functions, await makes it all look like synchronous JavaScript. As an added benefit, you can use try...catch with await instead of handling errors in ‘catch’ functions as you would have to if consuming Promises directly. The same code with await would look like this.

const url = 'https://the-one-api.dev/v2/book'
let res = await fetch(url) // Promise {<fullfilled>: Response} -await-> Response
try { let json = await res.json() console.log(json) // prints response, if no errors
} catch(err) { console.log(err) // prints error message, if any error
}

The async keyword is the other side of the same coin, in that it wraps any data to be sent within a Promise. Consider the following asynchronous function for adding several numbers. In the real world, your code would be doing something much more complicated.

async function sum(...nums) { return nums.reduce((agg, val) => agg + val, 0)
} sum(1, 2, 3) // Promise {<fulfilled>: 6}
 .then(res => console.log(res) // prints 6

let res = await sum(1, 2, 3) // Promise {<fulfilled>: 6} -await-> 6
console.log(res) // prints 6

These new features just the tip of the iceberg. We have barely even scratched the surface. JavaScript is constantly evolving, and new features are added to the language every year. It’s tough to keep up with the constant barrage of new features and idioms introduced to the language manually.

Wouldn’t it be nice if some tool could handle this for us? Fret not, there is. We’ve already talked in detail about setting up static code analysis in your JavaScript repo using ESLint. It’s extremely useful and should be an indispensable tool of your toolchain. But to be honest, setting up ESLint auto-fix pipelines and processes takes time and effort. Unless you enjoy this sort of plumbing, you’d be better off if you wrote the code and outsourced the plumbing to…DeepSource!

DeepSource can help you with automating the code reviews and save you a ton of time. Just add a .deepsource.toml file in the root of the repository and DeepSource will pick it up for scanning right away. The scan will find scope for improvements across your code and help you fix them with helpful descriptions.

Originally published on DeepSource Blog.

Guidelines for Java code reviews

Saif Sadiq — Mon, 12 Apr 2021 05:57:10 +0000

Having another pair of eyes scan your code is always useful and helps you spot mistakes before you break production. You need not be an expert to review someone’s code. Some experience with the programming language and a review checklist should help you get started. We’ve put together a list of things you should keep in mind when you’re reviewing Java code. Read on!

1. Follow Java code conventions

Following language conventions helps quickly skim through the code and make sense of it, thereby improving readability. For instance, all package names in Java are written in lowercase, constants in all caps, variable names in CamelCase, etc. Find the complete list of conventions here.

Some teams develop their own conventions, so be flexible in such cases!

2. Replace imperative code with lambdas and streams

If you’re using Java 8+, replacing loops and extremely verbose methods with streams and lambdas makes the code look cleaner. Lambdas and streams allow you to write functional code in Java. The following snippet filters odd numbers in the traditional imperative way:

List<Integer> oddNumbers = new ArrayList<>();
for (Integer number : Arrays.asList(1, 2, 3, 4, 5, 6)) {
    if (number % 2 != 0) {
      oddNumbers.add(number);
  }
}

This is the functional way of filtering odd numbers:

List<Integer> oddNumbers = Stream.of(1, 2, 3, 4, 5, 6)
  .filter(number -> number % 2 != 0)
  .collect(Collectors.toList());

3. Beware of the `NullPointerException`

When writing new methods, try to avoid returning nulls if possible. It could lead to null pointer exceptions. In the snippet below, the highest method returns a null if the list has no integers.

class Items {
    private final List<Integer> items;
    public Items(List<Integer> items) {
            this.items = items;
    }
    public Integer highest() {
      if (items.isEmpty()) return null;
      Integer highest = null;
      for (Integer item : items) {
          if (items.indexOf(item) == 0) highest = item;
          else highest = highest > item ? highest : item;
      }
      return highest;
    }
}

Before directly calling a method on an object I recommend checking for nulls as shown below.

Items items = new Items(Collections.emptyList());
Integer item = items.highest();
boolean isEven = item % 2 == 0; // throws NullPointerException ❌
boolean isEven = item != null && item % 2 == 0 // ✅

It can be pretty cumbersome to have null checks everywhere in your code though. If you are using Java 8+, consider using the Optional class to represent values that may not have valid states. It allows you to easily define alternate behavior and is useful for chaining methods.

In the snippet below, we are using Java Stream API to find the highest number with a method which returns an Optional. Note that we are using Stream.reduce, which returns an Optional value.

public Optional<Integer> highest() {
    return items
            .stream()
            .reduce((integer, integer2) -> 
                            integer > integer2 ? integer : integer2);
}
Items items = new Items(Collections.emptyList());
items.highest().ifPresent(integer -> {             // ✅
    boolean isEven = integer % 2 == 0;
});

Alternatively, you could also use annotations such as @Nullable or @NonNull which will result in warnings if there is a null conflict while building the code. For instance, passing a @Nullable argument to a method that accepts @NonNull parameters.

4. Directly assigning references from client code to a field

References exposed to the client code can be manipulated even if the field is final. Let’s understand this better with an example.

private final List<Integer> items;
public Items(List<Integer> items) {
        this.items = items;
}

In the above snippet, we directly assign a reference from the client code to a field. The client can easily mutate the contents of the list and manipulate our code as shown below.

List<Integer> numbers = new ArrayList<>();
Items items = new Items(numbers);
numbers.add(1); // This will change how items behaves as well

Instead, consider cloning the reference or creating a new reference and then assigning it to the field as shown below:

private final List<Integer> items;
public Items(List<Integer> items) {
    this.items = new ArrayList<>(items);
}
}

The same rule applies while returning references. You need to be cautious so as not to expose internal mutable state.

5. Handle exceptions with care

While catching exceptions, if you have multiple catch blocks, ensure that the sequence of catch blocks is most specific to least. In the snippet below, the exception will never be caught in the second block since the Exception class is the mother of all exceptions.

try {
    stack.pop();
} catch (Exception exception) {
    // handle exception
} catch (StackEmptyException exception) {
    // handle exception
}

If the situation is recoverable and can be handled by the client (the consumer of your library or code) then it is good to use checked exceptions. e. g. IOException is a checked exception that forces the client to handle the scenario and in case the client chooses to re-throw the exception then it should be a conscious call to disregard the exception.

6. Ponder over the choice of data structures

Java collections provide ArrayList, LinkedList, Vector, Stack, HashSet, HashMap, Hashtable. It’s important to understand the pros and cons of each to use them in the correct context. A few hints to help you make the right choice:

Map: Useful if you have unordered items in the form of key, value pairs and require efficient retrieval, insertion, and deletion operations. HashMap, Hashtable, LinkedHashMap are all implementations of the Map interface.
List: Very commonly used to create an ordered list of items. This list may contain duplicates. ArrayList is an implementation of the List interface. A list can be made thread-safe using Collections.synchronizedList thus removing the need for using Vector. Here’s some more info on why Vector is essentially obsolete.
Set: Similar to list but does not allow duplicates. HashSet implements the Set interface.

7. Think twice before you expose

There are quite a few access modifiers to choose from in Java — public, protected, private. Unless you want to expose a method to the client code, you might want to keep everything private by default. Once you expose an API, there’s no going back.

For instance, you have a class Library that has the following method to checkout a book by name:

public checkout(String bookName) {
    Book book = searchByTitle(availableBooks, bookName);
  availableBooks.remove(book);
  checkedOutBooks.add(book);
}

private searchByTitle(List<Book> availableBooks, String bookName) {
...
}

If you do not keep the searchByTitle method private by default and it ends up being exposed, other classes could start using it and building logic on top of it that you may have wanted to be part of the Library class. It could break the encapsulation of the Library class or it may be impossible to revert/modify later without breaking someone else’s code. Expose consciously!

8. Code to interfaces

If you have concrete implementations of certain interfaces (e. g. ArrayList or LinkedList) and if you use them directly in your code, then it can lead to high coupling. Sticking with the List interface enables you to switch over the implementation any time in the future without breaking any code.

public Bill(Printer printer) {
    this.printer = printer;
}

new Bill(new ConsolePrinter());
new Bill(new HTMLPrinter());

In the above snippet, using the Printer interface allows the developer to move to another concrete class HTMLPrinter.

9. Don’t force fit interfaces

Take a look at the following interface:

interface BookService {
        List<Book> fetchBooks();
    void saveBooks(List<Book> books);
    void order(OrderDetails orderDetails) throws BookNotFoundException, BookUnavailableException;   
}

class BookServiceImpl implements BookService {
...

Is there a benefit of creating such an interface? Is there a scope for this interface being implemented by another class? Is this interface generic enough to be implemented by another class? If the answer to all these questions is no, then I’d definitely recommend avoiding this unnecessary interface that you’ll have to maintain in the future. Martin Fowler explains this really well in his blog.

Well then, what’s a good use case for an interface? Let’s say we have a class Rectangle and a class Circle that has behavior to calculate perimeter. If there is a requirement, to sum up, the perimeter of all shapes — a use case for polymorphism, then having the interface would make more sense, as shown below.

interface Shape {
        Double perimeter();
}

class Rectangle implements Shape {
//data members and constructors
    @Override
    public Double perimeter() {
        return 2 * (this.length + this.breadth);
    }
}

class Circle implements Shape {
//data members and constructors
    @Override
    public Double perimeter() {
        return 2 * Math.PI * (this.radius);
    }
}

public double totalPerimeter(List<Shape> shapes) {
    return shapes.stream()
               .map(Shape::perimeter)
               .reduce((a, b) -> Double.sum(a, b))
               .orElseGet(() -> (double) 0);
}

10. Override hashCode when overriding equals

Objects that are equal because of their values are called value objects. e. g. money, time. Such classes must override the equals method to return true if the values are the same. The equals method is usually used by other libraries for comparison and equality checks; hence overriding equals is necessary. Each Java object also has a hash code value that differentiates it from another object.

class Coin {
    private final int value;

    Coin(int value) {
        this.value = value;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Coin coin = (Coin) o;
        return value == coin.value;
    }
}

In the above example, we have overridden only the equals method of Object.

HashMap<Coin, Integer> coinCount = new HashMap<Coin, Integer>() {{
  put(new Coin(1), 5);
  put(new Coin(5), 2);
}};

//update count for 1 rupee coin
coinCount.put(new Coin(1), 7);

coinCount.size(); // 3 🤯 why?

We would expect coinCount to update the number of 1 rupee coins to 7 since we override equals. But HashMap internally checks if the hash code for 2 objects is equal and only then proceeds to test equality via the equals method. Two different objects may or may not have the same hash code but two equal objects must always have the same hash code, as defined by the contract of the hashCode method. So checking for hash code first is an early exit condition. This implies that both equals and hashCode methods must be overridden to express equality.

If you write or review Java code, DeepSource can help you with automating the code reviews and save you a ton of time. Just add a .deepsource.toml file in the root of the repository and DeepSource will pick it up for scanning right away. The scan will find scope for improvements across your code and help you fix them with helpful descriptions.

Python code review checklist

Saif Sadiq — Tue, 30 Mar 2021 13:22:22 +0000

As developers, we are all too familiar with code reviews. Having another pair of eyes take a look at our code can be wonderful; it shows us so many aspects of our code we would not have noticed otherwise. A code review can be informative, and it can be educational. I can confidently attribute most of what I know about good programming practices to code reviews.

The amount of learning a reviewee takes away from a code review depends on how well the review is performed. It thus falls on the reviewer to make their review count by packing the most lessons into the review as possible.

This is a guide on some vital aspects of the code you should be checking in your reviews, the expectations you should have from those checks, and some ideas on how tooling (such as linters, formatters, and test suites) can help streamline the process.

Code review checklist

We have formulated this guide in the form of a checklist. It lists a set of questions that you need to ask about the code. If the answer to any of them is not a ‘yes’, you should leave a remark on the PR.

Do note that this list is only meant to serve as a guideline. Your codebase, like every codebase, has its own specific set of needs, so feel free to build upon this guide and override pieces of it that do not fit well for your use-cases.

Etiquettes

The first and foremost thing to check during a review is how closely the PR adheres to basic etiquettes. Good PRs are composed of bite-sized changes and solve a single well-defined problem. They should be focused and purposefully narrow to have as few merge conflicts as possible. Put simply, a good PR facilitates being reviewed.

For large-scale swooping changes, make a separate target branch and then make small incremental PRs to that target, finally merging the target with the main branch. Making one humongous PR makes it harder to review, and if it goes stale, many merge conflicts may pop up.

When reviewing PRs from new developers, I also make it a point to ensure their commit messages are well-written.

Checklist:

Is the PR atomic?
Does the PR follow the single concern principle?
Are the commit messages well-written?

Ideas:

Enforce a commit message format in the team. For example, you could try gitmoji wherein you use emoji in commit messages. For example, bugfixes should start with [FIX], or the 🐛 emoji and new features should start with [FEAT] or the ✨ emoji. This makes the intention of the commit very clear.

Functionality and syntax

The next thing to check is whether the PR is effective in that it works. Code changed by the PR should work as expected. A bug fix should solve the bug it was supposed to fix. A feature should provide the additional functionality that was required without breaking something else.

An important thing to keep in mind is that any new feature added by a PR is justified. A simple way to ensure this is to accept only the PRs that are associated with an already triaged issue. This practice minimizes feature-creep.

Checklist:

Does the PR work?
Does the new feature add value or is it a sign of feature-creep?
Does the PR add test-cases for the modified code?

Ideas:

Having comprehensive tests in the code makes it easier to check that new functionality works and harder for PRs to break existing stuff. The portion of the code covered should never go down in a PR. Any new code should come with complete coverage in unit/functional tests. Python comes with a robust unit testing framework built into the language itself. You should make use of it in your codebase.

Design

Once we’ve established that the code works, the next step is to check how well it integrates with the existing codebase. One key thing to inspect in this regard is duplication. Often, code added to a repo provides functionality that might already be a part of the code in a different location or something provided by the frameworks or Pip packages in use. This knowledge is something that one can only gain by experience. As a senior, it becomes your duty to point such duplication out to new developers contributing to your repo.

Attention to paradigms is also essential. Many projects have pre-adopted design paradigms such as microservices, mono repo, or cloud-nativity. Any incoming code should be in line with these paradigms.

Checklist:

Is the code properly planned and designed?
Will the code work well with the existing code and not increase duplication?
Is the code well organised in terms of placement of components?

Patterns and idioms

Python is a very mature language. It is driven based on certain philosophies, described as the Zen of Python. Those philosophies have given birth to many conventions and idioms that new code is expected to follow.

The difference between idiomatic and non-idiomatic code is very subtle, and in most cases, can only be intuitively gauged. As with all things honed by experience, intuition must be transferred from experienced people, like yourself, to newbies like your reviewees.

Checklist:

Does the code keep with the idioms and code patterns of the language?
Does the code make use of the language features and standard libraries?

Ideas:

Most linters, popularly PyLint, can help you identify deviations from the style guides and, in most cases, even automatically fix them. Linters work incredibly fast and can make corrections to the code in real-time, making them a valuable addition to your toolchain.

Readability

Python is widely regarded as a very readable language. Python’s simplicity of syntax and reduced usage of punctuation contribute heavily to its readability. It only makes sense for code written in the language to be readable as well.

Checklist:

Is the code clear and concise?
Does it comply with PEP-8?
Are all language and project conventions followed?
Are identifiers given meaningful and style guide-compliant names?

Ideas:

A good code formatter like Black can help a lot in formatting the code for consistency and readability. Black also offers minimal customization, which is good because it eliminates all forms of bikeshedding.

We’ve previously talked about integrating Black into your CI pipeline can work wonders during a code review.

Documentation and maintainability

The next thing to check is the maintainability of the code. Any code added or changed by the PR should be written to facilitate someone other than the original author to maintain it.

It should preferably be self-documenting, which means written in a way that anyone reading the code may be able to understand what it does. This is one of the hallmarks of good Python code. If the code has to be complex by design, it should be amply documented. In an ideal world, all classes and functions would have Python docstrings, complete with examples.

Checklist:

Is the code self-documenting or well-documented?
Is the code free of obfuscation and unnecessary complexity?
Is the control flow and component relationship clear to understand?

Ideas:

Sphinx is a documentation generator that exports beautiful documentation from Python docstrings. The exported documentation can then be uploaded to ReadTheDocs, a popular doc-hosting tool. Sphinx is one of the main reasons why I absolutely love writing documentation.

Security

Ensuring that the application remains secure is critical. The next thing to check is if the PR maintains or improves the security of the project. You need to ensure that the changes do not increase the attack surface area or expose vulnerabilities. If the PR adds new dependencies, they could potentially be unsafe, in which case you might need to check the version for known exploits and update the dependencies, if necessary.

Checklist:

Is the code free of implementation bugs that could be exploited?
Have all the new dependencies been audited for vulnerabilities?

Ideas:

One of the renowned security analyzers for Python is Bandit. Also, if you use GitHub for hosting code, you should absolutely read this guide about setting up vulnerability detection and Dependabot for your codebase.

Once you set up vulnerability detection on GitHub, you’ll get notifications like this…

… with details about the vulnerability and PRs in your repositories that you can merge to patch them.

We’ve also recently talked about common security pitfalls in Python development and how you can secure your projects against them.

Performance, reliability and scalability

The final things to check are the performance and reliability of the code at scale. While these are undoubtedly key metrics, I put them on the bottom of the checklist because I believe well-planned, well-designed, and well-written code generally works well too.

Checklist:

Is the code optimised for in terms of time and space complexity?
Does it scale as per the need?
Does it have instrumentation like reporting for metrics and alerting for failures?

Ideas:

An excellent way to have add some reliability to Python is to use type hinting and static type checking that can provide hints into possible errors before runtime. Python new native support for type-hints is primarily inspired by the Mypy syntax, which can be incrementally adopted in existing projects.

DeepSource

DeepSource is an automated code review tool that manages the end-to-end code scanning process and automatically makes pull requests with fixes whenever new commits are pushed or new pull requests.

Setting up DeepSource for Python is a quick, easy, no-fuss process. Just add a .deepsource.toml file in the root of the repo, and immediately DeepSource will pick it up for scanning. The scan will find scope for improvements across your codebase, make those improvements, and open pull requests for the changes it finds. I was blown away by the simplicity of the setup and the efficacy of their self-built code engine.

🙋‍♂️ Trivia: Did you know that DeepSource holds the distinction of being on OWASP’s curated list of source code analysis tools for Python?

The ideal code review

Returning to my point about code reviews being educational and informative, I would also like to add that code reviews are time-consuming and resource-intensive. While each review takes time, the more in-depth and comprehensive ones consume even more.

So each review must be as productive as possible. This is where all the tooling and automation efforts should be directed. By automating whatever can be automated, which is generally the mundane parts of the review, such as code style and formatting, we can allow devs to focus on the important stuff like architecture, design, and scalability.

I’ll leave you with a profound thought my mentor shared with me.

A good code review not only improves the code but the coder as well.

Common anti-patterns in Go

Saif Sadiq — Wed, 24 Mar 2021 06:05:42 +0000

It has been widely acknowledged that coding is an art, and like every artisan who crafts wonderful art and is proud of them, we as developers are also really proud of the code we write. In order to achieve the best results, artists constantly keep searching for ways and tools to improve their craft. Similarly, we as developers keep levelling up our skills and remain curious to know the answer to the single most important question - “How to write good code”. 🤯

Frederick P. Brooks in his book “ The Mythical Man Month: Essays on Software Engineering ” wrote :

“The programmer, like the poet, works only slightly removed from pure thought-stuff. He builds his castles in the air, from air, creating by exertion of the imagination. Few media of creation are so flexible, so easy to polish and rework, so readily capable of realizing grand conceptual structures.

Image source: https://xkcd.com/844/

This post tries to explore answers to the big question mark in the comic above. The simplest way to write good code is to abstain from including anti-patterns in the code we write.😇

What are anti-patterns? 🤔

Anti-patterns occur when code is written without taking future considerations into account. Anti-patterns might initially appear to be an appropriate solution to the problem, but, in reality, as the codebase scales, these come out to be obscure and add ‘technical debt’ to our codebase.

A simple example of an anti-pattern is to write an API without considering how the consumers of the API might use it, as explained in example 1 below. Being aware of anti-patterns and consciously avoid using them while programming is surely a major step towards a more readable and maintainable codebase. In this post, let’s take a look at a few commonly seen anti-patterns in Go.

1. Returning value of unexported type from an exported function

In Go, to export any field or variable we need to make sure that its name starts with an uppercase letter. The motivation behind exporting them is to make them visible to other packages. For example, if we want to use the Pi function from math package, we should address it as math.Pi . Using math.pi won’t work and will error out.

Names (struct fields, functions, variables) that start with a lowercase letter are unexported, and are only visible inside the package they are defined in.

An exported function or method returning a value of an unexported type may be frustrating to use since callers of that function from other packages will have to define a type again to use it.

// Bad practice
type unexportedType string
func ExportedFunc() unexportedType { return unexportedType("some string")
} // Recommended
type ExportedType string
func ExportedFunc() ExportedType { return ExportedType("some string")
}

2. Unnecessary use of blank identifier

In various cases, assigning value to a blank identifier is not needed and is unnecessary. In case of using the blank identifier in for loop, the Go specification mentions :

If the last iteration variable is the blank identifier, the range clause is equivalent to the same clause without that identifier.

// Bad practice
for _ = range sequence
{ run()
} x, _ := someMap[key] _ = <-ch // Recommended
for range something
{ run()
} x := someMap[key] <-ch

3. Using loop/multiple `append`s to concatenate two slices

When appending multiple slices into one, there is no need to iterate over the slice and append each element one by one. Rather, it is much better and efficient to do that in a single append statement.

As an example, the below snippet does concatenation by appending elements one by one through iterating over sliceTwo.

for _, v := range sliceTwo { sliceOne = append(sliceOne, v)
}

But, since we know that append is a variadic function and thus, it can be invoked with zero or more arguments. Therefore, the above example can be re-written in a much simpler way by using only one append function call like this:

sliceOne = append(sliceOne, sliceTwo…)

4. Redundant arguments in `make` calls

The make function is a special built-in function used to allocate and initialize an object of type map, slice, or chan. For initializing a slice using make, we have to supply the type of slice, the length of the slice, and the capacity of the slice as arguments. In the case of initializing a map using make, we need to pass the size of the map as an argument.

make, however, already has default values for those arguments:

For channels, the buffer capacity defaults to zero (unbuffered).
For maps, the size allocated defaults to a small starting size.
For slices, the capacity defaults to the length if capacity is omitted.

Therefore,

ch = make(chan int, 0)
sl = make([]int, 1, 1)

can be rewritten as:

ch = make(chan int)
sl = make([]int, 1)

However, using named constants with channels is not considered as an anti-pattern, for the purposes of debugging, or accommodating math, or platform-specific code.

const c = 0
ch = make(chan int, c) // Not an anti-pattern

5. Useless `return` in functions

It is not considered good practice to put a return statement as the final statement in functions that do not have a value to return.

// Useless return, not recommended
func alwaysPrintFoofoo() { fmt.Println("foofoo") return
} // Recommended
func alwaysPrintFoo() { fmt.Println("foofoo")
}

Named returns should not be confused with useless returns, however. The return statement below really returns a value.

func printAndReturnFoofoo() (foofoo string) { foofoo := "foofoo" fmt.Println(foofoo) return
}

6. Useless `break` statements in `switch`

In Go, switch statements do not have automatic fallthrough. In programming languages like C, the execution falls into the next case if the previous case lacks the break statement. But, it is commonly found that fallthrough in switch-case is used very rarely and mostly causes bugs. Thus, many modern programming languages, including Go, changed this logic to never fallthrough the cases by default.

Therefore, it is not required to have a break statement as the final statement in a case block of switch statements. Both the examples below act the same.

Bad pattern:

switch s {
case 1: fmt.Println("case one") break
case 2: fmt.Println("case two")
}

Good pattern:

switch s {
case 1: fmt.Println("case one")
case 2: fmt.Println("case two")
}

However, for implementing fallthrough in switch statements in Go, we can use the fallthrough statement. As an example, the code snippet given below will print 23.

switch 2 {
case 1: fmt.Print("1") fallthrough
case 2: fmt.Print("2") fallthrough
case 3: fmt.Print("3")
}

7. Not using helper functions for common tasks

Certain functions, for a particular set of arguments, have shorthands that can be used instead to improve efficiency and better understanding/readability.

For example, in Go, to wait for multiple goroutines to finish, we can use a sync.WaitGroup. Instead of incrementing a sync.WaitGroup counter by 1 and then adding -1 to it in order to bring the value of the counter to 0 and in order to signify that all the goroutines have been executed :

wg.Add(1) // ...some code
wg.Add(-1)

It is easier and more understandable to use wg.Done() helper function which itself notifies the sync.WaitGroup about the completion of all goroutines without our need to manually bring the counter to 0.

wg.Add(1)
// ...some code
wg.Done()

8. Redundant `nil` checks on slices

The length of a nil slice evaluates to zero. Hence, there is no need to check whether a slice is nil or not, before calculating its length.

For example, the nil check below is not necessary.

if x != nil && len(x) != 0 { // do something
}

The above code could omit the nil check as shown below:

if len(x) != 0 { // do something
}

9. Too complex function literals

Function literals that only call a single function can be removed without making any other changes to the value of the inner function, as they are redundant. Instead, the inner function that is being called inside the outer function should be called.

For example:

fn := func(x int, y int) int { return add(x, y) }

Can be simplified as:

10. Using `select` statement with a single case

The select statement lets a goroutine wait on multiple communication operations. But, if there is only a single operation/case, we don’t actually require select statement for that. A simple send or receive operation will help in that case. If we intend to handle the case to try a send or receive without blocking, it is recommended to add a default case to make the select statement non-blocking.

// Bad pattern
select {
case x := <-ch: fmt.Println(x)
} // Recommended
x := <-ch
fmt.Println(x)

Using default:

select {
case x := <-ch: fmt.Println(x)
default: fmt.Println("default")
}

11. context.Context should be the first param of the function

The context.Context should be the first parameter, typically named ctx. ctx should be a (very) common argument for many functions in a Go code, and since it’s logically better to put the common arguments at the first or the last of the arguments list. Why? It helps us to remember to include that argument due to a uniform pattern of its usage. In Go, as the variadic variables may only be the last in the list of arguments, it is hence advised to keep context.Context as the first parameter. Various projects like even Node.js have some conventions like error first callback. Thus, it’s a convention that context.Context should always be the first parameter of a function.

// Bad practice
func badPatternFunc(k favContextKey, ctx context.Context) { 
    // do something
}

// Recommended
func goodPatternFunc(ctx context.Context, k favContextKey) {    
    // do something
}

When it comes to working in a team, reviewing other people’s code becomes important. DeepSource is an automated code review tool that manages the end-to-end code scanning process and automatically makes pull requests with fixes whenever new commits are pushed or new pull requests.

Setting up DeepSource for Go is extremely easy. As soon as you have it set up, an initial scan will be performed on your entire codebase, find scope for improvements, fix them, and open pull requests for those changes.

go build

Breaking builds, baseball bats, and the code quality DNA

Saif Sadiq — Mon, 15 Mar 2021 13:07:43 +0000

“What’s the biggest set of items in terms of tech debt. I can attack it, scope it into the code quality sprints and then start incrementally evolving” - Badri elaborated.

In Good Code Podcast, we talk to experienced developers and technical leaders about the art and science of writing good code and building software. In episode 5, we talk to Badri Rajasekar, who has spent almost 20 years building software and is currently the founder and CEO of Jamm - a rapidly growing video collaboration app.

The Microsoft executive who alarmed everyone with a baseball bat

While speaking about code quality and code review processes, Badri shared an incident from his initial days at Microsoft, when somebody broke the Windows Build and had hundreds of developers waiting for it to get fixed. Soon enough, a Microsoft executive stormed into the office with a baseball bat shouting “Who broke the build?”

With this hilarious (not at the time 😅) incident, Badri expounded on how code quality reviews have evolved over the past decade. Earlier, with strongly typed languages and no automation in place, errors could easily become blunders. Today, with the introduction of automated code reviews, various tooling, microservices, the concept of nightlies, and continuous integrations, there’s much more tolerance for both errors and blunders.

Quick Read: Code Review Best Practices

The flip side: With the flexibility that these modern processes offer, it becomes even more critical to be careful in order to ensure code quality at every step of the way.

In Badri’s words:

It’s just as possible for organizations in this era of continuous delivery to ship a lot of buggy code.

The takeaway: It’s extremely important to be structured around automation testing.

Slow down today and move 10X faster tomorrow onwards

While talking about how inexperienced developers or new startups that don’t bother about integration testing for the sake of quick deployments, end up accumulating a lot of tech debt before they realize it, Badri shared another example of a rapidly growing startup that couldn’t slow down, hence making the problem worse for itself every day.

Badri’s advice for situations like this - get as organized as possible, and try to not fix everything in one shot.

Have a strong process around all of your basic unit tests. Integration tests have to pass before a pull request gets merged in. Eg. Don’t submit a PR until the integration tests have passed. Be firm - either the tests have to pass or if they’re not passing, you’ve got to fix the test.

The key is to introduce code quality into the DNA of the company and encourage everyone on the team to think of it as an integral part of the process, and not rely on QA teams or site reliability engineers. Badri also shared his insights on how to do this — introducing quality sprints, having regular performance checks, taking care of the meta-questions, and maintaining documentation.

He spoke about his time at TokBox and how they used to manage the code quality. They used to have a unified framework for instrumenting everything from platform health to analytics. They also made it fun by introducing the “Load Time Lama” - a fanciful creature who encouraged everyone to take care of the loading times.

Pro Tip: Avoid premature optimization of architecture, as you’ll miss out on crucial problems that haven’t cropped up since you haven’t seen scale yet.

The similarity between pilots and programmers

Badri drew an analogy between pilots and programmers while explaining how to steer through massive amounts of data. When flying an airplane, a pilot majorly looks at a couple of indicators — the airspeed indicator, altimeter, heading indicator, etc. And when one of such indicators indicates a warning, only then he delves deeper into the issues.

Similarly, while having analytics and continuous testing are critical, developers should not get lost in the data that accumulates over time. The way out is to simply stick to the basics. Have a “four-metric dashboard” that defines the code quality benchmarks for a team.

At the same time, practicing causality where you reverse engineer the issues, rather than looking at a sea of graphs, makes it easier to figure out causal patterns.

The conversation wrapped up with golden advice:

Golden Advice: Keep your architecture simple!

Badri advocated the KISS (keep it simple silly) principle when it comes to architectures. This improves scalability and helps with faster resolution of unforeseen problems, because these are easy to reason about.

After a lot of anecdotes, jokes, advisements, and laughter, the chat concluded.

Interested in listening to the entire conversation?

Listen to the podcast here.

Stay tuned for the next episode of the Good Code Podcast!

Automated Code Review Tools for Developers in 2021

Saif Sadiq — Mon, 08 Mar 2021 13:02:30 +0000

Code review is a technique that can improve the quality of a codebase by having multiple developers look for bugs and other problems before passing them on to others. Manual code reviews are costly and time-consuming, which is why many development teams use automated tools to do this work.

Automated code review tools can help you automate the process, improve your code quality, and save valuable developer time. Developers want to focus on building their applications instead of reviewing other people’s code. Automated code review tools have been around for a while, but they’ve been evolving and getting better. They are now more efficient, accurate, and customizable than ever before.

Why Use Automated Code Review Tools?

Automated code review tools have been around for a while as static analysis and unit testing frameworks. However, as business needs require speed and agility, it’s necessary to automate code review. It can lead to faster feedback, better code quality, and time to production.

This blog will explore the top automated code review tools in 2021 and help you choose which one is best for your needs by looking at the pros and cons of each.

CodeBeat

CodeBeat is a popular code review tool that provides automated code review and feedback. It displays a code grade on a ‘4.0 scale’ system where the code gets reviewed on a scale of 1 to 4. CodeBeat supports various languages like Python, Ruby, Java, Javascript, Golang, Swift, and more.

CodeBeat offers a team management tool that makes it easy to analyze the code and move developers within a team while maintaining consistency. Integrating with many popular toolings like Github, Gitlab, Bitbucket, Slack, and Hipchat, developers and software teams use CodeBeat at scale.

Some of the highlights of CodeBeat includes:

Provides an integrated dashboard with project reviews
Issues grouped into categories like complexity, code issues, and duplication
Provides E-Mail updates for project and pull request quality continuously
Provides immediate feedback to improve the codebase quality through “quick wins”
Easy to integrate and use with minimal setup required

Some of the drawbacks of CodeBeat are:

Lack of security analysis.
Lack of support for open-source tools and linters.

CodeBeat is entirely free for open-source, with enterprise support offered for large teams. CodeBeat provides a great degree of analysis for identifying cyclomatic complexity, thus identifying duplicated code.

DeepSource

DeepSource is an automated code review tool that provides automated code analysis against various popular general-purpose programming languages. DeepSource supports languages like Python, Javascript, Golang, Ruby, and Java. With its single-file configuration, DeepSource makes continuous analysis easy with every commit and pull request.

The code quality measures check for performance issues, type check issues, style issues, documentation issues, bug risks, and anti-patterns. It allows us to define clear and realistic goals for developers and maintainers to manage their codebases and make code review easier.

Some of the highlights of DeepSource includes:

Single-File Configuration for automated code analysis
Integrates with continuous integration pipelines like Travis CI and Circle CI
Supports code formatters like black, rubocop, and gofmt
Provides auto-fix for common issues across the codebase
Provides analysis for every issue and pull request

Some of the drawbacks of DeepSource are:

Lack of support for PHP, C++, and Rust
Lack of support for Azure DevOps

DeepSource is entirely free for open-source projects, with enterprise support for large teams. DeepSource analyzers work at file-level and repository-level and provide a low positive rate in comparison to other analyzers and code review tools.

CodeClimate

CodeClimate is a code review tool that aims to improve team productivity by bringing commit-to-deploy visibility. It aims to ease up continuous delivery with “Velocity”, which provides Engineering Intelligence, while “Quality” provides automated code review on every commit and pull request.

CodeClimate provides a maintainability score on a scale from A to F depending on various parameters, including code duplications, code smells, and more. It allows us to identify bottlenecks and provides trends like the change in test coverage or technical debt.

Some of the highlights of CodeClimate includes:

Easy installation with automated Git updates.
Identifies hotspots in the codebase to identify portions that need a refactor.
Provides a security dashboard to identify application vulnerabilities.
Provides an API to be used locally for automated code reviews.
Provides alerts and instance notifications over Mail and RSS feeds.

Some of the drawbacks of CodeClimate are:

Lack of issue description and search/filtering.
Lack of customization capabilities and high pricing.

CodeClimate suffers from a high rate and does not provide rules for identifying core complexities like file length and cognitive complexity. CodeClimate also integrates with Integrated Development Environments (IDE) like VS Code and Atom. It also features a library called “cc-test-reporter” to test the coverage.

Codacy

Codacy is one of the most popular automated code review tools used by individual developers and software development teams alike. Codacy supports various general-purpose programming languages like Python, Java, Javascript, C/C++, Ruby, Golang, and more.

Codacy covers code complexity, error-prone, security, code style, compatibility, documentation, and performance issues.

Some of the highlights of Codacy includes:

Automated code reviews with minimal installation
Integration with various services including GitHub, GitLab, GitHub Actions, CircleCI
Help define particular goals for the Project and provides a recommendation to fulfill them
Analyzes pull requests and commit individually
Only new issues are taken into account to prevent noise and duplication

Some of the drawbacks of Codacy are:

Lack of issue search apart from a few filters
Lacks support for exporting code patterns

Codacy provides an easy-to-use and intuitive user interface that can help developers to manage their code fluidly. It allows the developers to keep the code quality intact and the code review clean.

Veracode

Veracode is an automated code review tool for code review, automated testing, and improving codebase efficiency. Supporting various general-purpose programming languages like Python, Java, Javascript, Golang, and more, Veracode provides two code review tools: static analysis and software composition analysis.

The static analysis tool leverages static analysis, where developers can find bugs and anti-patterns and fix them before they land into production. The Software Composition Analysis allows identifying vulnerabilities while using third-party packages in the codebase.

Some of the highlights of Veracode includes:

Easy to configure and quick to use.
Provides binary scanning to have less false positives in the code.
Pin-points to real vulnerabilities in the code and recommends solutions.
Intuitive and friendly User-Interface with custom dashboards

Some of the drawbacks of Veracode are:

Lack of customization for analysis rules
Lack of a well-defined user experience

Veracode’s code analysis platform enables developers to review, analyze, and remediate code to find security vulnerabilities. Veracode also provides SDLC integration, which helps developers verify compliance with the OWASP Top 10 and other best practices.

Conclusion

Automated code review tools are a boon for developers. There’s no shortage of options available, and this article explored 5 tools, each with its advantages and disadvantages.

5 Common mistakes in Go

Saif Sadiq — Mon, 08 Mar 2021 06:49:22 +0000

Bug-risks are issues in code that can cause errors and breakages in production. A bug is a flaw in the code that produces undesired or incorrect results. Code often has bug-risks due to poor coding practices, lack of version control, miscommunication of requirements, unrealistic time schedules for development, and buggy third-party tools. In this post, let’s take a look at a few commonly seen bug-risks in Go.

1. Infinite recursive call

A function that calls itself recursively needs to have an exit condition. Otherwise, it will recurse forever until the system runs out of memory.

This issue can be caused by common mistakes such as forgetting to add an exit condition. It can also happen “on purpose.” Some languages have tail-call optimization, which makes certain infinite recursive calls safe to use. Tail-call optimization allows you to avoid allocating a new stack frame for a function because the calling function will return the value that it gets from the called function. The most common use is tail-recursion, where a recursive function written to take advantage of tail-call optimization can use constant stack space. Go, however, does not implement tail-call optimization, and you will eventually run out of memory. However, this problem doesn’t apply to spawning new goroutines.

Recommended reading: Why is a Goroutine’s stack infinite ?

2. Assignment to `nil` map

A map needs to be initialized using the make function (or a map literal) before you can add any elements. A new, empty map value is made using the built-in function make, which takes the map type and an optional capacity hint as arguments:

make(map[string]int)
make(map[string]int, 100)

The initial capacity does not bound its size: maps grow to accommodate the number of items stored in them, with the exception of nil maps. A nil map is equivalent to an empty map except that no elements may be added.

Bad pattern:

var countedData map[string][]ChartElement

Good pattern:

countedData := make(map[string][]ChartElement)

Recommended reading: Go: assignment to entry in nil map

3. Method modifies receiver

A method that modifies a non-pointer receiver value may have unwanted consequences. This is a bug risk because the method may change the value of the receiver inside the method, but it won’t reflect in the original value. To propagate the change, the receiver must be a pointer.

For example:

type data struct { num int key *string items map[string]bool
} func (d data) vmethod() { d.num = 8
} func (d data) run() { d.vmethod() fmt.Printf("%+v", d) // Output: {num:1 key:0xc0000961e0 items:map[1:true]}
}

If num must be modified:

type data struct { num int key *string items map[string]bool
} func (d *data) vmethod() { d.num = 8
} func (d *data) run() { d.vmethod() fmt.Printf("%+v", d) // Output: &{num:8 key:0xc00010a040 items:map[1:true]}
}

4. Possibly undesired value being used in goroutine

Range variables in a loop are reused at each iteration; therefore, a goroutine created in a loop will point to the range variable from the upper scope. This way, the goroutine could use the variable with an undesired value.

In the example below, the value of index and value used in the goroutine are from the outer scope. Because the goroutines run asynchronously, the value of index and value could be (and usually are) different from the intended value.

mySlice := []string{"A", "B", "C"}
for index, value := range mySlice { go func() { fmt.Printf("Index: %d\n", index) fmt.Printf("Value: %s\n", value) }()
}

To overcome this problem, a local scope must be created, like in the example below.

mySlice := []string{"A", "B", "C"}
for index, value := range mySlice { index := index value := value go func() { fmt.Printf("Index: %d\n", index) fmt.Printf("Value: %s\n", value) }()
}

Another way to handle this could be by passing the values as args to the goroutines.

mySlice := []string{"A", "B", "C"}
for index, value := range mySlice { go func(index int, value string) { fmt.Printf("Index: %d\n", index) fmt.Printf("Value: %s\n", value) }(index, value)
}

5. Deferring `Close` before checking for a possible error

It’s a common pattern amongst Go developers, to defer the Close() method for a value that implements the io.Closer interface. For example, when opening a file:

f, err := os.Open("/tmp/file.md")
if err != nil { return err
}
defer f.Close()

But this pattern is harmful for writable files because deferring a function call ignores its return value, and the Close() method can return errors. For instance, if you wrote data to the file, it might have been cached in memory and not flushed to disk by the time you called Close. This error should be explicitly handled.

While you could go ahead without using defer at all, you would need to remember to close the file everytime their job is done. A better way would be to defer a wrapper function, like in the example below.

f, err := os.Open("/tmp/file.md")
if err != nil { return err
} defer func() { closeErr := f.Close() if closeErr != nil { if err == nil { err = closeErr } else { log.Println("Error occured while closing the file :", closeErr) } }
}()
return err

Recommended reading: Do not defer Close() on writable files

go build!

Leveraging static code analysis in a Ruby CI pipeline

Saif Sadiq — Tue, 02 Mar 2021 17:18:06 +0000

Continuous integration, or CI, refers to the culture and the technologies that enable continuously merging features and bug fixes into the main branch of the codebase. Code changes are incorporated immediately after testing, rather than being bunched with other updates in a waterfall release process.

Similarly, continuous delivery, or CD, refers to automatically deploying the changed code to the target environment, such as pre-production branches to staging and master to production. CD picks up where CI left off, and as such, they often go hand in hand.

Static code analysis typically falls under the CI aspect of a CI/CD pipeline. Taking the example of a small Ruby project, we’ll be setting up a CI workflow to analyze code quality using static analysis in the following areas:

Consistency, with the widely adopted Ruby style guide.
Layout, such as unjust spacing or misaligned indentation.
Linting, such as inadequate permissions or redundant operations.
Security analysis, such as the use of unsafe methods.

Prerequisites

Creating a sandbox

Let’s make a fresh new directory for our adventure today. Initialize a Git repository in the folder and check it into GitHub as we’ll be using GitHub Workflows as our CI tool (more on this later).

$ mkdir proj
$ cd proj/
$ touch .gitignore

Setting up Ruby and Bundler

You probably already have Ruby installed on your computer. But I find it best not to use pre-installed Ruby for a couple of reasons. Reason the first, it’s generally much older than the latest stable version, and you don’t want to miss out on Ruby’s newest features, do you? Reason the second, it’s relatively easy to break your system by installing, removing, or updating a critical package.

Don’t fret; there’s a solution — RVM. I won’t get into details of RVM here, but using it, you can install and manage several versions of Ruby on a system, keeping the system Ruby pristine.

$ rvm install 3.0.0
$ rvm use 3.0.0

Next, we set up Bundler, a fantastic package manager for Ruby. We need it to keep track of our projects dependencies. It’s extremely straightforward to install.

$ gem install bundler
$ bundle init

To ensure that our project gems remain localized to our project, we can set up Bundler to install Gems at a given path. Create a directory .bundle/ and a config file within the directory with the following content.

With this configuration, Bundler will install all gems inside a .gems/ folder inside the current project folder proj/. Add both directories .bundle/ and .gems/ to your .gitignore file so that they are not checked into VCS.

Getting familiar with Rubocop

For analyzing the code quality in all the areas we mentioned above, we will be using Rubocop, one of the finest linters available for Ruby. Rubocop comes with an extensive collection of rules, called ‘cops’, organized in groups, called ‘departments’, based on their functionality.

To install, add the line to your Gemfile and run bundle install.

# frozen_string_literal: true source 'https://rubygems.org' git_source(:github) { |repo_name| "https://github.com/#{repo_name}" } + gem 'rubocop', '~> 1.9', require: false

To list all the offences in any given file or directory, just pass the names as arguments to rubocop.

$ rubocop <file/dir_name>

Rubocop is also capable of autocorrecting most of the errors it reports, which is incredibly helpful. To enable autocorrection, pass the -a flag. Passing -A uses a more aggressive auto-correct mode, which is not advisable unless you are sure of what you are doing.

$ rubocop -a <file/dir_name> # safe autocorrect, recommended
$ rubocop -A <file/dir_name> # unsafe autocorrect, not recommended

You will need to prepend bundle exec to these commands if Rubocop is not globally installed.

The code

Now we get to the fun part, scripting in Ruby. Take this script, for example. It takes a file name as an argument and prints said file’s content to STDOUT, very similar to the cat command (hence the name).

# cat.rb
filename = ARGV[0] file = open(filename)
list = file.read
file.close list.each_line.with_index{|line| puts line
}

It is formatted poorly, violates many rules from the style guide, and even has a couple of gaping security flaws. We’ll fix those pretty soon but first, let’s do a preliminary scan with Rubocop and observe the output.

$ bundle exec rubocop cat.rb
Inspecting 1 file
W Offenses: cat.rb:1:1: C: [Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
# cat.rb
^
cat.rb:4:8: C: Security/Open: The use of Kernel#open is a serious security risk.
file = open(filename) ^^^^
cat.rb:8:16: W: [Correctable] Lint/RedundantWithIndex: Remove redundant with_index.
list.each_line.with_index{|line| ^^^^^^^^^^
cat.rb:8:26: C: [Correctable] Layout/SpaceBeforeBlockBraces: Space missing to the left of {.
list.each_line.with_index{|line| ^
cat.rb:8:26: C: [Correctable] Layout/SpaceInsideBlockBraces: Space between { and | missing.
list.each_line.with_index{|line| ^^
cat.rb:8:26: C: [Correctable] Style/BlockDelimiters: Avoid using {...} for multi-line blocks.
list.each_line.with_index{|line| ^
cat.rb:10:2: C: [Correctable] Layout/TrailingEmptyLines: Final newline missing.
} 1 file inspected, 7 offenses detected, 6 offenses auto-correctable

Rubocop found 7 offenses, of which 6 it can correct automatically, labeled as [Correctable] in the output above.

Pipeline

Picking the infrastructure

Choices abound when it comes to picking a CI/CD infrastructure provider. From Travis CI, a darling of open-source developers, to Jenkins, the tool of choice for enterprise teams who’d rather self-host their customized solution, dev-ops engineers are spoilt for choice.

But the simplest of these in my experience has been GitHub workflows, a GitHub-native solution allowing you to set up entire chains of jobs, described as YAML files, that can be initiated based on specific triggers. We can use them throughout the CI/CD pipeline, from running checks on PRs before merge to deploying the code after. There are hundreds of pre-built actions (many officially maintained) that take the effort out of setting up end-to-end pipelines.

Naturally, we’ll be using GitHub workflows as our CI pipeline infrastructure. The end goal is to have linting as a check on our PRs and commits. Only PRs that pass the checks would be mergeable.

Adding lint workflow

Let’s see what the workflow file would look like in our case. Create a new directory .github/, create another directory within this one named workflows/, and in this directory, create a file named lint.yml.

# .github/workflows/lint.yml
name: Lint

on:
 push:
 branches:
 - master

jobs:
 lint:
 name: Lint
 runs-on: ubuntu-latest
 steps:
 - uses: actions/checkout@v2
 - uses: ruby/setup-ruby@v1
 with:
 ruby-version: 3.0
 bundler-cache: true # runs `bundle install`
 - run: bundle exec rubocop .

The lint workflow consists of a single job. The job is fired on every push event to the master branch and performs three steps:

actions/checkout: checks out the code repository
ruby/setup-ruby:
1. sets up Ruby version 3.0 and the latest compatible version of Bundler
2. uses Bundler to install all packages in the Gemfile
run: runs Rubocop on the entire current working directory

Once the job completes, we get our outcome. In our case, it’s a big red cross. The workflow execution fails because Rubocop found issues in the code. The logs reveal the same message we had seen earlier, and lists offences identified by Rubocop.

😢 We’ll get there, eventually.

Fixing problems

We want our check to pass. Nobody likes failing checks. Coming back to our local setup, let’s use Rubocop’s autofix feature to quickly resolve these issues. First, we should let Rubocop take care of the automatic stuff using the -a flag.

$ bundle exec rubocop -a cat.rb
Inspecting 1 file
W Offenses: cat.rb:1:1: C: [Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
# cat.rb
^
cat.rb:4:8: C: Security/Open: The use of Kernel#open is a serious security risk.
file = open(filename) ^^^^
cat.rb:8:15: C: [Corrected] Layout/ExtraSpacing: Unnecessary spacing detected.
list.each_line do |line| ^
cat.rb:8:16: W: [Corrected] Lint/RedundantWithIndex: Remove redundant with_index.
list.each_line.with_index{|line| ^^^^^^^^^^
cat.rb:8:26: C: [Corrected] Layout/SpaceBeforeBlockBraces: Space missing to the left of {.
list.each_line.with_index{|line| ^
cat.rb:8:26: C: [Corrected] Layout/SpaceInsideBlockBraces: Space between { and | missing.
list.each_line.with_index{|line| ^^
cat.rb:8:26: C: [Corrected] Style/BlockDelimiters: Avoid using {...} for multi-line blocks.
list.each_line.with_index{|line| ^
cat.rb:10:2: C: [Corrected] Layout/TrailingEmptyLines: Final newline missing.
} 1 file inspected, 8 offenses detected, 6 offenses corrected, 1 more offense can be corrected with `rubocop -A`

Rubocop solved most of the issues reported, including one issue introduced during the autofix process itself! With 6 of the 7 problems are already fixed, we’ve managed to shave off ~70% of our work with zero effort input.

What’s left is one unsafe autofix and one security vulnerability that Rubocop cannot fix automatically. We can take care of those:

The frozen string literal comment is missing. That’s a reasonable thing to add to the file, so we’ll let Rubocop add it using the stronger autofix flag -A.

$ bundle exec rubocop -A cat.rb
Inspecting 1 file
C Offenses: cat.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
# cat.rb
^
cat.rb:2:1: C: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
# cat.rb
^
cat.rb:6:8: C: Security/Open: The use of Kernel#open is a serious security risk.
file = open(filename) ^^^^ 1 file inspected, 3 offenses detected, 2 offenses corrected

- file = open(filename)
+ file = File.open(filename)

Commit and push. We’re green now! At this point, you should pat yourself on the back for a job well done.

🥳 Yay!

Staying clean

Now that you’re at peak code quality, we need to ensure it stays that way. This means that we need to ensure that no PR negatively affects our codebase quality. To run the check on every incoming PR, add the pull_request event to our lint workflow.

on: on: push: branches: - master
+ pull_request:
+ branches:
+ - master

Now, to test that our check is working as expected, we need to make a PR with some code that Rubocop would flag. Let’s refactor the lines in our script, that are concerned with reading the file, to use a block.

# cat.rb # frozen_string_literal: true filename = ARGV[0] - file = File.open(filename)
- list = file.read
- file.close
+ File.open(filename) do |file|
+ list = file.read
+ end

list.each_line do |line|
  puts line
end

Check out a new branch from master. Commit and push to this branch and open a PR. You’ll see that the checks fail, and thus the PR cannot be merged unless overridden by an administrator.

🚧 Our check is working just fine!

🧠 Brain-teaser: Can you identify why the updated code, using a block, is being flagged by Rubocop?

🤷‍♂️ Hint: If you want a hint, here’s the Rubocop output for the PR:

$ bundle exec rubocop cat.rb
Inspecting 1 file
W

Offenses:

cat.rb:7:3: W: Lint/UselessAssignment: Useless assignment to variable - list.
  list = file.read
  ^^^^

1 file inspected, 1 offense detected

🧑‍💻 Answer: Defining list inside the block means that it is not accessible outside the block. This makes the assignment useless and will lead to a bug in the subsequent use of the variable.

💡 Lesson: Though indirectly, code analysis can sometimes also help identify potential bugs!

DeepSource

While we invested considerable time and effort, we now have checks and actions set up to monitor code quality in our repo. But what if we didn’t have time to spare or didn’t want to spend the effort? We’re busy developers, after all!

Consider using DeepSource. It continuously scans the code on every commit, and on every pull request, through various static code analyzers (including linters and security analyzers), and can automatically fix some of them. DeepSource also has its custom-built analyzers for most languages that are constantly improved and kept up-to-date.

It’s incredibly easy to set up! You need only add a .deepsource.toml file in your repository root, and DeepSource will pick it up. It takes much less effort and the end result is way more polished that setting up several workflows in GitHub.

version = 1

[[analyzers]]
name = "ruby"
enabled = true

[[transformers]]
name = "rubocop"
enabled = true

Automate the tedium away

CI/CD pipelines are quintessential to the agile development workflow. The ability to add features, squash bugs, and get the changes in production instantly can make a very significant difference. Startups live or die based on how often they iterate.

Integrating static analysis into the CI pipeline ensures that only the cleanest and most compliant code makes its way into production. For something that takes very little time to set up, consumes a minuscule amount of resources, and does not significantly affect test/build timings, static analysis can add a lot of confidence to your build process.

Confident iterations await. Till next time!

Automate code formatting in Python

Saif Sadiq — Tue, 16 Feb 2021 08:06:53 +0000

Right off the bat, let’s clarify an important distinction. Writing code that works and writing good code are two very different things. The former is a skill while the latter is an art form, and this difference distinguishes great programmers from the crowd.

When we talk of good code, the word ‘good’ is vague by design. That’s because there are no rules set in stone about what makes code good or bad. All we have are some abstract guidelines such as readability:

Programs are meant to be read by humans and only incidentally for computers to execute.

– Abelson & Sussman

Those are two MIT professors with pretty solid credentials. Identifying the quality of code is an intuition that is honed over time, through practice and experience. Code reviews go a long way towards this goal.

Code review is the process where developers more experienced than yourself read through your code and suggest improvements that could make it better. These suggestions can improve performance, incorporate new language features, patch security oversights, or correct code style.

... when it comes to code review.

But manual code reviews are expensive. The time it takes someone to read your code is time they did not spend building awesome stuff. They are also error-prone and by no means comprehensive. There are human limits to knowledge and memory.

Enter automation. Automated code reviews are faster, less error-prone, and more in-depth than their manual counterparts.

Let’s dive deep into the process with a sample project containing a single Python file. We’ll riddle the file with issues and then set up a workflow that automatically finds and fixes these problems.

Ingredients

A. Codebase with PEP-8 violations

Before we automate code review, let’s first write some code to review. Here is a sample program that lists primes up to a given number. It might hurt to look at, which is good as it means your code-olfactory senses are working.

# list_primes.py
def is_prime (number): for i in range(2, number): if number%i==0: return False return True def list_primes(upper): for number in range(2, upper): if is_prime(number): print(F"{number} is prime") list_primes(10)

There are a lot of problems with this script. Don’t get me wrong, it works, but it’s not good.

Extraneous space before function parenthesis
2-space indentation
No spaces around operators
Single line around functions
Uppercase F in f-strings

B. Black code formatter

Black is a popular code formatter for Python. It is capable of automatically reformatting your Python files, fixing all code style violations. What’s neat is that it is pretty opinionated and can’t be configured much, making it ideal for automation.

So let’s install Black, while also taking the opportunity to set up some first-class dependency management with a tool I personally love, Pipenv. Running the following command creates two files, Pipfile and Pipfile.lock, in the root of the repo and installs Black as a dev dependency.

$ pipenv install --pre --dev black

Running Black without any args formats all files in the repo directly. Apart from reformatting your files, it has two less dangerous modes.

--check: In this mode, Black purely checks if there are any code style violations. The return code is 0 if there are no violations and non-zero if there are any.

$ pipenv run black --check list_primes.py
would reformat /Users/dhruvkb/Documents/scratch/automata/list_primes.py
Oh no! 💥 💔 💥
1 file would be reformatted.

--diff: In this mode, Black shows the changes it will make without actually making them. This mode is helpful if you want to inspect the changes before they are actually made.

$ pipenv run black --diff list_primes.py
--- list_primes.py  2020-02-02 00:00:00.000000 +0000
+++ list_primes.py  2020-02-02 00:00:00.000000 +0000
@@ -1,17 +1,19 @@
 # list_primes.py
-def is_prime (number):
+def is_prime(number):
- for i in range(2, number):
+ for i in range(2, number):
- if number%i==0:
+ if number % i == 0:
- return True
+ return True

- return False
+ return False
+
 def list_primes(upper): for number in (2, 10): if is_prime(number):
- print(F"{number} is prime")
+ print(f"{number} is prime")
+

 list_primes(15)

Recipe

Code review can be split in two parts: the interesting part where you solve big picture issues and the mundane parts where you identify non-idiomatic snippets and code style violations. Let’s automate the boring parts of the code review.

1. Set up Git hooks

We just saw how incredible Black is. Wouldn’t it be awesome if Black ran automatically every time you were to commit your code? It’s possible, with Git hooks. Git hooks are programs that run on your codebase when you execute certain Git commands. The ‘pre-commit’ hook is of particular interest to us because we’d like the lint check to take place before the commit is created, and prevent the commit from being created if it fails.

Autohooks is a Python package for managing these hooks via Python. It has a plugin system that enables integration with tools like Black. Let’s install both Autohooks and the Black-integration plugin.

$ pipenv install --dev autohooks autohooks-plugin-black

Make a pyproject.toml file in the root of your repo with the following content.

[tool.autohooks]
mode = "pipenv"
pre-commit = ["autohooks.plugins.black"]

[tool.autohooks.plugins.black]
arguments = ["--check"]

Activate the hooks and run the check function to see if everything works fine.

$ pipenv run autohooks activate
✓ autohooks pre-commit hook installed at /Users/hal/Documents/scratch/.git/hooks/pre-commit using pipenv mode.

$ pipenv run autohooks check
✓ autohooks pre-commit hook is active.
✓ autohooks pre-commit hook is up-to-date.
ℹ Using autohooks mode "pipenv".
✓ Plugin "autohooks.plugins.black" active and loadable.

Try git commit-ing the poorly written source code. Ha, gotcha! Here’s how things will go down:

The pre-commit hook will be initiated.
The top-level hooks by Autohooks will be invoked.
Autohooks will then execute Black with the --check argument.
Black will return a non-zero code because the file contains errors.
Git will halt the commit operation.

$ git add . && git commit -m "Add the source code to VCS"
ℹ autohooks => pre-commit
ℹ Running autohooks.plugins.black
would reformat /Users/dhruvkb/Documents/scratch/automata/list_primes.py
Oh no! 💥 💔 💥
1 file would be reformatted.
×

💡 Pro-tip: You can bypass the hook with the --no-verify flag on git commit. It’s not recommended but we’re not the police, so do what you want.

💡 Pro-tip: You can remove the --check argument and then every time you commit, Black will reformat your files for you.

2. Lint check using GitHub Actions

The main drawback of Git hooks is that they are local. In a project with multiple contributors, there might be people who might forget to activate the hook or actively try to bypass them. In such cases, the solution is to run the lint check on the remote repo itself. GitHub Actions provides an extremely versatile solution for running the lint.

Create the file lint.yml inside the .github/workflows directory of the repo.

# .github/workflows/lint.yml
name: Lint

on: [push, pull_request]

jobs:
 lint:
 runs-on: ubuntu-latest
 steps:
 - uses: actions/checkout@v2
 - uses: actions/setup-python@v2
 - uses: psf/black@stable

This workflow checks out the repository, sets up Python, installs Black and then lints the files. By default, this action runs Black with the --check and --diff arguments. Once you set up linting, all future commits and PRs will pass through Black.

Our list_primes.py file will fail the test. The logs will show both the failing files as well as the diffs for those files (because of the -diff argument). That’ll come in pretty handy when you’re fixing the violations.

3. Lint fixes using GitHub Workflows

That brings us to the one aspect we still haven’t addressed yet. Black is capable of reformatting files, but so far, we have only used it to detect issues and present diffs. We’ve not tapped into Black’s full potential yet.

How about we turn the automation up to 11 and update our GitHub workflow to automatically fix code style violations?

# .github/workflows/lint.yml
name: Lint

on: - push
- workflow_dispatch

jobs:
 lint:
 runs-on: ubuntu-latest
 steps:
 - uses: actions/checkout@v2
 - uses: actions/setup-python@v2
 - name: Install Python dependencies
 run: |
 pip install pipenv
 pipenv install --deploy --dev --system
 - uses: wearerequired/lint-action@v1
 with:
 github_token: ${{ secrets.GITHUB_TOKEN }}
 black: true
 auto_fix: true

This workflow uses the same two steps as the previous one, that is checking out the repo and setting up Python. Then we install Pipenv and use that to install Black on the system. The lint-action action runs Black and then commits the changed files. This creates a new commit with the same changes that Black had shown in the diff!

💡 Pro-tip: You can customize the author’s name and email and also the message of the commit. Just add the following to the with key of the action. It’s an opportunity to be creative!

commit_message: 'Opening the pod-bay doors'
git_name: 'HAL-9000'
git_email: 'hal@hal.hal'

Now that you can be assured that code pushed to the repo is free from style guide deviations, this frees up code reviewers to take a bigger picture look at the code and leave the minutiae to HAL.

End-to-end automation with DeepSource

Phew! That was a lot of work, wasn’t it? But guess what, we only looked at style guide violations for now. Adding more features like looking for security gaps, finding possible bugs, and making complex refactors would make this a very long exercise. But it does not have to be.

You could also consider automating this entire audit, review and refactor process with DeepSource that can scan your code on every commit, and for every pull request, through several tools (including linters and security analyzers) and can automatically fix many issues. DeepSource also has its custom-built analyzers for most languages that are constantly improved and kept up-to-date.

It’s incredibly easy to set up! You need only add a .deepsource.toml file in your repository root, and DeepSource will pick it up. Much less effort than what we just went through.

version = 1

[[analyzers]]
name = "python"
enabled = true

  [analyzers.meta]
  runtime_version = "3.x.x"
  max_line_length = 80

[[transformers]]
name = "black"
enabled = true

Finis coronat opus

Code reviews are a very important learning tool for new developers. It’s a way to transfer knowledge, experience, and convention from a senior developer to a junior, a way to understand how even the code that was deemed as final can be made better, cleaner, and more efficient.

I’d venture so far as to say that code reviews are one of the best learning tools for developers, and they are wasted on mundane things like code style. Introduce a little automation and make every code review count.

They know enough who know how to learn.

When done well, a code review can be a truly educational experience. Automation cannot replace that. What automation can do is take the mundane out of the process.

Here’s to better code reviews!

Common Python security pitfalls, and how to avoid them

Saif Sadiq — Wed, 10 Feb 2021 17:43:05 +0000

Python is undoubtedly a popular language. It consistently ranks among the most popular and most loved languages year after year. That’s not hard to explain, considering how fluent and expressive it is. Its pseudocode-like syntax makes it extremely easy for beginners to pick it up as their first language, while its vast library of packages (including the likes of giants like Django and TensorFlow) ensure that it scales up for any task required of it.

Being such a widely-used language makes Python a very attractive target for malicious hackers. Let’s see a few simple ways to secure your Python apps and keep the black-hats at bay.

Problems and solutions

Python places a lot of importance on zen, or developer happiness. The clearest evidence of that lies in the fact that the guiding principles of Python are summarized in a poem. Try import this in a Python shell to read it. Here are some security concerns that might disturb your zen, along with solutions to restore it to a state of calm.

Unsafe deserialization

OWASP Top Ten, a basic checklist for web security, mentions unsafe deserialization as one of the ten most common security flaws. While it’s common knowledge that executing anything coming from the user is a terrible idea, serializing and deserializing user input does not seem equally serious. After all, no code is being run, right? Wrong,

PyYAML is the de-facto standard for YAML serialization and deserialization in Python. The library supports serializing custom data types to YAML and deserializing them back to Python objects. See this serialization code here and the YAML produced by it.

# serialize.py
import yaml class Person: def __init__ (self, name, age): self.name = name self.age = age def __str__ (self): return f'<Person: {self.name} - {self.age}>' def __repr__ (self): return str(self) person = Person('Dhruv', 24)
with open('person.yml', 'w') as output_file: yaml.dump(person, output_file)

!!python/object: __main__.Person
age: 24
name: Dhruv

Deserializing this YAML gives back the original data type.

# deserialize.py
import yaml # class Person needs to be present in the scope
with open('person.yml', 'r') as input_file: person = yaml.load(input_file, Loader = yaml.Loader) print(person)

$ python deserialize.py ↵
<Person: Dhruv - 24>

As you can see, the line !!python/object: __main__.Person in the YAML describes how to re-instantiate objects from their text representations. But this opens up a slew of attack vectors, that can escalate to RCE when this instantiation can execute code.

Solution

The solution, as trivial as it may seem, is to use safe-loading by swapping out the loader yaml.Loader in favor of the yaml.SafeLoader loader. This loader is safer because it completely blocks loading of custom classes.

# deserialize.py
import yaml # class Person needs to be present in the scope
with open('person.yml', 'r') as input_file: person = yaml.load(input_file, Loader = yaml.SafeLoader) print(person)

$ python deserialize.py ↵
ConstructorError: could not determine a constructor for the tag 'tag:yaml.org,2002:python/object: __main__.Person'
  in "person.yml", line 1, column 1

Standard types like hashes and arrays can still be serialized to and deserialized from YAML documents just like before. Most people, probably including you, won’t even realize the difference.

age: 24
name: Dhruv

$ python deserialize.py ↵
{'age': 24, 'name': 'Dhruv'}

Dynamic execution

Python has a pair of very dangerous functions: exec and eval. Both are very similar in terms of what they do: process the strings passed to them as Python code. exec expects the string to be a statement, which it will execute and not return a value. eval expects the string to be an expression and will return the computed value of the expression.

Here is an example of what both these functions in action.

eval('2 + 5') # returns 7
exec('print("Hello")') # prints "Hello", no return

You could in theory, pass a statement to eval and get a similar effect as exec, because in Python returning None is virtually the same as not returning anything at all.

eval('print("Hello")') # prints "Hello", returns None

The danger of these functions lies in the ability of these functions to execute virtually any code in the same Python process. Passing any input to the function that you cannot be 100% certain about, is akin to handing over your server keys to malicious hackers on a plate. This is the very definition of RCE.

Solution

There are ways to mitigate the access that eval has. You can restrict access to globals and locals by passing dictionaries as the second and third arguments to eval respectively. Remember that locals take priority over globals in case of conflict.

x = 3
eval('x + 5') # returns 8
eval('x + 5', { 'x': 2 }) # returns 7
eval('x + 5', { 'x': 2 }, { 'x': 1 }) # returns 6

That makes the code safer yes. At least it somewhat prevents the data in the variables from being leaked.

But it still doesn’t prevent the string from accessing any built-ins like pow, or more dangerously, __import__. To counter that, you need to override __builtins__.

eval(" __import__ ('math').sqrt(5)", {}, {}) # returns 2.2360679774997898
eval( " __import__ ('math').sqrt(5)", { " __builtins__": None }, {} # restricts access to built-ins
) # error

Safe to expose now? Not quite. Because regardless of how secure you make eval, it’s job is to evaluate an expression and nothing can stop the expression from taking too long and freezing the server for a long time.

eval("2 ** 2147483647", { " __builtins__": None }, {}) # goodbye!

As we Python developers like to say, “eval is evil.”

Dependency management

Python’s popularity draws the attention of white-hat security researchers just as much as it does that of hackers with malicious intent. As a result, new security vulnerabilities that are constantly discovered, disclosed, and patched. To keep the malicious hackers at bay, your software needs to keep all its dependencies up to date.

A common technique for pinning packages in Python is the ubiquitous requirements.txt file, a simple file that lists all the dependencies and exact versions needed by your project.

Let’s say you install Django. As of writing, Django depends on three more packages. If you freeze your dependencies, you end up with the following requirements. Note that only one of these dependencies was installed by you, the other 3 are sub-dependencies.

$ pip freeze ↵
asgiref==3.3.1
Django==3.1.5
pytz==2020.5
sqlparse==0.4.1

pip freeze does not place dependencies in levels and that’s a problem. For smaller projects with a few dependencies that you can keep a track of mentally, this is not a big deal but as your projects grow, so will your top level dependencies. Conflicts arise when sub-dependencies overlap. Updating individual dependencies is a mess too, because the graph relationship of these dependencies is not clear from the plain text file.

Solution

Pipenv and Poetry are two tools that help you manage dependencies better. I prefer Pipenv but Poetry is equally good. Both package managers build on top of pip. Fun fact: DeepSource is compatible with both Pipenv and Poetry as package managers.

Pipenv, for example, tracks your top-level dependencies in a Pipfile and then does the hard work of locking down dependencies in a lockfile name Pipfile.lock, similar to how npm manages Node.js packages. Here’s an example Pipfile.

[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

[dev-packages]

[packages]
django = "*"

[requires]
python_version = "3.9"

With this, you can get a clear picture of the top-level dependencies of your app. Updating dependencies is also much easier because you just need to update the top level packages and the locking algorithm will figure out the most compatible and up-to-date versions of all the sub-dependencies.

Here is the same example with Django. Notice how Pipenv can identify your top-level dependencies and its dependencies and so on.

$ pipenv graph ↵
Django==3.1.5
  - asgiref [required: >=3.2.10,<4, installed: 3.3.1]
  - pytz [required: Any, installed: 2020.5]
  - sqlparse [required: >=0.2.2, installed: 0.4.1]

If your code is hosted on GitHub, make sure you turn on and configure Dependabot as well. It’s a nifty little bot that alerts you if any of your dependencies has gone out of date or if a vulnerability has been identified in the pinned version of a dependency. Dependabot will also make PRs to your repo, automatically updating your packages. Very handy indeed!

Runtime assertions

Python has a special assert keyword for guarding against unexpected situations. The purpose of assert is simple, verify a condition and raise an error if the condition is not fulfilled. In essence, assert evaluates the given expression and

if it evaluates to a truthy value, moves along
if it evaluates to a falsey value, raises an AssertionError with the given message

Consider this example.

def do_something_dangerous(user, command): assert user.has_permissions(command), f'{user} is not authorized' user.execute(command)

This is a very simple example where we check if the user has the permissions to perform an action and subsequently perform the given action. In this case, if user.has_permissions() returns False, our assertion would cause an AssertionError and the execution would be halted. Seems pretty safe, right?

No. Assertions are tools for developers during development and debugging phases. Asserts should not be used to guard critical functionality. The Python constant, __debug__ is set to False during compilation, which removes assert statements from the compiled code to optimise the code for performance. Removing assert statements from the compiled code leaves the function unguarded.

Another reason to avoid asserts is that assertion errors are not helpful to debuggers as they provide no information other than the fact that an assertion did not hold up. Defining apt exception classes and then raising their instances is a much more solid solution.

Solution

For an alternative approach, go back to the basics. Here’s the same program, this time using if/else and a raising a PermissionError (you’ll need to define it somewhere) when the required assertion is unfulfilled.

def do_something_dangerous(user, command): if user.has_permissions(command): # [safer as it will not be removed by compiler user.execute(command) else: raise PermissionError(f'{user} is not authorized') # suitable error class

This code uses straightforward Python constructs, works the same with __debug__ set to True or False and raises clear exceptions that can be handled with much more clarity.

Achieving Zen

The key takeaway from all of these examples is to never trust your users. Input provided by the users should not be serialized-deserialized, evaluated, executed or rendered. To be safe, you must be careful of what you write and thoroughly audit the code after its been written.

Do you know what’s better than scanning for vulnerabilities in code after it’s been written? Getting vulnerabilities highlighted as soon as you write code with them.

Bandit

Static Code Analysis tools such as linters and vulnerability scanners can help you find a lot of issues before they get exploited in the wild. An excellent tool for finding security vulnerabilities in Python is Bandit. Bandit goes through each file, generates an abstract syntax tree (AST) for it and then runs a whole slew of tests on this AST. Bandit can detect a whole bunch of vulnerabilities out-of-the-box and can also be extended for specific scenarios and compatibility with frameworks via plugins. As a matter of fact, Bandit is capable of detecting all of the aforementioned security shortcomings.

If you’re a Python developer, I cannot recommend Bandit enough. I could write a whole article extolling its virtues. I might even do that.

DeepSource

You should also consider automating this entire audit and review process using code review automation tools like DeepSource that scans your code, on every commit and for every PR, through it’s linters and security analyzers and can automatically fix a multitude of issues. DeepSource also has its own custom-built analyzers for most languages that are constantly improved and kept up-to-date. And it’s incredibly easy to set up!

version = 1

[[analyzers]]
name = "python"
enabled = true

  [analyzers.meta]
  runtime_version = "3.x.x"
  max_line_length = 80

Who knew it could be so simple?

Experience the zen of Python, and be careful not to let black-hats disturb your peace!

5 JavaScript Static Analysis Tools

Saif Sadiq — Tue, 09 Feb 2021 14:58:39 +0000

With the rise of modern software development practices, the prominence of static analysis has grown. Static code analysis allows developers to improve the codebase's readability and consistency while finding possible bugs and anti-patterns. Static analysis tools help us to validate the modern development standards and assess the quality of the same.

A majority of the software development teams, across the world, have been using static code analysis tools. Static code analysis identifies bad and redundant code and fixes it before it lands in production. With automated static analysis, we don't need to rely on dynamic analysis, where the code gets executed on a processor to identify bugs.

Let's take a look at some of the best JavaScript static analysis tools that you can make use of.

Why Use Static Analysis for JavaScript?

The Javascript ecosystem covering almost every development need has gathered pace for the past few years. With static analysis, we can easily maintain the code quality with little effort.

Bugs and duplicated code is retrieved and fixed, with code insights generated at every stage of development. It allows the developers to keep track of the blockers they might face in the code and possibly fix them, thus eliminating a pain point.

DeepSource

DeepSource is one of the most popular tools for static analysis, providing tracking over 800+ potential issues, like unused variables, empty functions, usage of Script URLs, and more in JavaScript codebase. DeepSource JavaScript analyzer currently supports a wide variety of Javascript libraries and frameworks like ReactJS, VueJS, AngularJS, Angular, Ember, and more, along with various ECMAScript versions and Typescript. If you are following a style guide, DeepSource provides support for that as well, along with the module system.

DeepSource strictly enforces ESLint core JavaScript rules, which allow us to identify bugs, anti-patterns, and non-performant code.

Some key features of DeepSource include:

Single file configuration.
Highlight important metrics like documentation coverage and dependency.
Auto-fix for common issues.
Code metrics tracking and reporting.
Analysis of every pull request and commit.
Integrated dashboard with issue descriptions.

These features make DeepSource a lucrative choice for static analysis and provide a go-to-option for code analysis. With the analyzers working at file-level and repository-level, you would never have to worry in the future about maintaining your Javascript code.

DeepScan

DeepScan provides automated tracking of issues in the JavaScript codebase through static analysis. DeepScan supports a wide range of Javascript libraries and frameworks like React, Angular, and Vue and banks upon its data-flow analysis to find the code execution flow and issues. It also provides a general grade for the project you are working on to find ideas for improving the codebase quality.

Some key features of DeepScan includes:

Integrated dashboard for issue tracking.
Real-Time collaboration with team activity.
Active analysis over the codebase.
Usage of control flow graph for code execution.
Grade calculation through issue density tracking.

DeepScan is an active choice for developers and enterprise Teams for managing their code quality. Their static analysis goes beyond ESLint, providing more coverage and issue tracking, making it a definite choice to identify tricky issues.

LGTM

LGTM states its mission "to promote community-driven security analysis" and has made it possible through automated issue and vulnerability checking. LGTM banks upon CodeQL to drive its issue tracking and finding common bugs that occur across the codebase. LGTM supports an integrated dashboard for real-time analysis, along with issue personalization, to help teams focus on issues that matter to them.

LGTM supports various general-purpose programming languages, with Javascript being prominent. Its issue tracking is prioritized intelligently, which gives it a specific edge over other Static analysis Tools.

Some key features of LGTM are:

Provides SemmleQL to write our Code Analysis Queries.
Commits are checked every day.
Provides a REST API to integrate with the workflow.
Provides real-time project alerts and suppression.
Analyzed over original repositories, not forks.

LGTM banks upon intelligent detection, which is made possible by analyzing different codebases. Thus it highlights an alert if we introduce a new bug or vulnerability. With granular access to a user dashboard, LGTM is a definite go for maintainers looking for a specific holdover code analysis.

SonarCloud

SonarCloud is yet another static analysis tool that aims to champion quality code among software projects. Featuring an IDE extension, named SonarLint, and other features like bug and code smell detection and continuous inspection, SonarCloud is one of the favorite go-to tools. SonarCloud supports several general-purpose programming languages, with Javascript in prominence.

SonarCloud can be integrated with CI Pipelines like GitHub Actions and Azure DevOps, to ensure that bad code never lands in production.

Some key features of SonarCloud are:

Access to the project dashboard and the project metrics.
Features Go/No Go quality Gate while analyzing code.
Continuous inspection over all major Git providers.
IDE extension named SonarLint available for better use.
Supported by TravisCI, GitLabCI, CircleCI, and more.

The tool keeps track of code maintainability, reliability, coverage, and more while aiming to accelerate the reviews from maintainers. With security hotspots as an additional feature, it can provide broader coverage to help developers understand the issues and leaks.

Codacy

Codacy is one of the most popular static analysis tools providing coverage over issues such as code duplication, complexity, and more. Codacy is integrated with ESLint JavaScript linter to provide quick access to potential issues and bugs, on an integrated dashboard.

With Codacy, you can add specific repositories you have access to or have forked, and Codacy starts analyzing your code for bugs and style Issues.

Some key features of Codacy include:

Automated code reviews and issue tracking.
Integration with various Git providers.
Provides code standardization and user management.
Supported in various workflows and CI integrations.
Provides self-hosted services as well.

Codacy will re-analyze your source code with every push, which eases code reviews and analysis. With a supporting community, Codacy is gaining large traction among developers for code Reviews and analysis.

Java Code Quality Tools Recommended by Developers

Saif Sadiq — Mon, 08 Feb 2021 05:40:27 +0000

The best way to protect your Java code from avoidable bugs is to use static code analysis tools that can help you find and fix problematic code before it reaches production. Let's look at some popular static code analysis tools that can be used to test code from a number of different angles.

DeepSource

DeepSource delivers what is probably the best static code analysis you can find for Java. The DeepSource Java analyzer detects 190+ code quality issues, including performance bugs, security risks, bug risks, and anti-patterns. Currently, It supports Gradle Java projects, and in the future, DeepSource will add support for Maven and Android too. DeepSource is also working on bringing Autofix support to the Java analyzer, which will let developers fix issues without writing a single code line.

Features

Detects more than 170 code quality issues.
OpenJDK versions 8 to 14 are currently supported.

Integration: Gradle

Licensing: Free to use for open-source, Students, and Non-Profit Organisations. Paid plans starts from 12 USD user/month.

SonarQube

SonarQube is the open-source suite of java static code analysis tools that combines the features of tools such as FindBugs and PMD. SonarQube has very intuitive dashboards that maintain history to help developers track Java code quality over time. SonarQube uses advanced techniques like pattern matching and dataflow analysis to analyze code and identify code smells, bugs, and security vulnerabilities.

Features

It has 597 rules to detect various code quality issues.
Java language versions supported up-to 14.

Integration: Maven, Gradle, Ant.

Licensing: Community edition is free and open source. License for commercial editions starts at €120.

SpotBugs

SpotBugs is FindBugs' successor. It is a Java static code analysis tool that examines JVM bytecode and finds traces of potential errors and security vulnerabilities by identifying coding defects. These defects are reported as warnings, but not all of the warnings reported are necessarily defects, e.g., warnings referred to possible performance issues. The latest version reports more than 400 warnings, and all warnings are classified into four ranks: (i) scariest, (ii) scary, (iii) troubling, (iv) of concern.

Features

Detects more than 400 bug patterns in code.
SpotBugs requires JRE (or JDK) 1.8.0 or later to run. However, it can analyze programs compiled for any version of Java, from 1.0 to 1.9.

Integration: Ant, Maven, Gradle.

Licensing: Free and open-source

PMD

PMD analyzes Java source code, validates it with its list of rules, and reports offending lines to the user. PMD can determine common issues such as the hard coding of passwords and IP addresses, the use of the forEach loop instead of a traditional for loop, and code that seems to violate the Law of Demeter or implement the God Class anti-pattern.

Features

More than 250 rules to detect issues in java.
language support till Java 13

Integration: Maven, Gradle.

Licensing: Free and open-source

EclEmma

EclEmma(based on the JaCoCo library) is a free Java code coverage tool for Eclipse. It is a toolkit for measuring code coverage in a java code base and presenting coverage data through visual reports. It highlights the lines of code and the total percentage of code executed, and tracks both line and branch coverage. EclEmma helps developers assess code that has not been adequately tested and focuses on low coverage areas. It supports 3 types of report formats: HTML, XML & CSV.

Features

Supports Java class files from version 1.0 to 14.

Integration: Ant, Maven.

Licensing: Free and open-source

Checkstyle

Checkstyle is a java static analysis tool that helps developers automate the process of defining a style guide and enforcing coding standards within the enterprise. Checkstyle identifies rules that are violated and help them fix and reformat the code with IDEs such as Eclipse, IntelliJ IDEA or NetBeans. Categories of violations include wildcard imports and whitespace usage around generic tokens.

Features

More than 180 checks to enforce java coding style.
Language support till Java 14.

Integration: Ant, Maven.

Licensing: Free and open-source

JArchitect

JArchitect is a Java static analysis tool that evaluates code metrics such as the number of method parameters, variables and lines of code, cyclomatic complexity, afferent and efferent coupling, and so forth. It measures, queries, and visualizes your code and avoid unexpected issues, technical debt, and complexity.

Features

Language support from Java 8 to 13.
More than 450 rules.

Integration: Maven, Gradle, Ant.

Licensing: Free Trial for 14days, Open source license for Free for Non-commercial open source software projects, Personal License - $149, Developer Edition - $299, Build Machine - $549. Submit the form here to get the pertinent pricing and info for JArchitect.

JUnit

JUnit is a popular unit testing framework for Java development projects that allows developers to write and run unit tests for Java 8 and above. JUnit tests the state and the behavior of the code with simple yet powerful assertion statements. It is easy to get started with JUnit, and It offers a variety of additional features using annotations for more complex scenarios.

Features

JUnit 5(latest release) requires Java 8 (or higher) at runtime.

Integration: Maven, Gradle, Ant

Licensing: Free and open-source

Forem: Saif Sadiq

JavaScript best practices to improve code quality

1. Block scored declarations

2. Arrow functions

3. Optional chaining

4. Null-ish coalescing

5. Logical assignment

6. Named capture groups

7. async & await

Guidelines for Java code reviews

1. Follow Java code conventions

2. Replace imperative code with lambdas and streams

3. Beware of the NullPointerException

4. Directly assigning references from client code to a field

5. Handle exceptions with care

6. Ponder over the choice of data structures

7. Think twice before you expose

8. Code to interfaces

9. Don’t force fit interfaces

10. Override hashCode when overriding equals

Python code review checklist

Code review checklist

Etiquettes

Functionality and syntax

Design

Patterns and idioms

Readability

Documentation and maintainability

Security

Performance, reliability and scalability

DeepSource

The ideal code review

Common anti-patterns in Go

What are anti-patterns? 🤔

1. Returning value of unexported type from an exported function

2. Unnecessary use of blank identifier

3. Using loop/multiple appends to concatenate two slices

4. Redundant arguments in make calls

5. Useless return in functions

6. Useless break statements in switch

7. Not using helper functions for common tasks

8. Redundant nil checks on slices

9. Too complex function literals

10. Using select statement with a single case

11. context.Context should be the first param of the function

Breaking builds, baseball bats, and the code quality DNA

The Microsoft executive who alarmed everyone with a baseball bat

Slow down today and move 10X faster tomorrow onwards

The similarity between pilots and programmers

Automated Code Review Tools for Developers in 2021

Why Use Automated Code Review Tools?

CodeBeat

DeepSource

CodeClimate

Codacy

Veracode

Conclusion

5 Common mistakes in Go

1. Infinite recursive call

2. Assignment to nil map

3. Method modifies receiver

4. Possibly undesired value being used in goroutine

5. Deferring Close before checking for a possible error

Leveraging static code analysis in a Ruby CI pipeline

Prerequisites

Creating a sandbox

Setting up Ruby and Bundler

Getting familiar with Rubocop

The code

Pipeline

Picking the infrastructure

Adding lint workflow

Fixing problems

Staying clean

DeepSource

Automate the tedium away

Automate code formatting in Python

Ingredients

A. Codebase with PEP-8 violations

B. Black code formatter

7. `async` & `await`

3. Beware of the `NullPointerException`

3. Using loop/multiple `append`s to concatenate two slices

4. Redundant arguments in `make` calls

5. Useless `return` in functions

6. Useless `break` statements in `switch`

8. Redundant `nil` checks on slices

10. Using `select` statement with a single case

2. Assignment to `nil` map

5. Deferring `Close` before checking for a possible error