You are viewing an old version of this page. View the current version.

Compare with Current View Page History

Version 1 Current »

Apache Geode uses the LGTM.com GitHub App during pull request pre-checkin to analyze code submissions for a variety of security and code quality issues. The progress of the analysis is shown along with other pre-checkin tests at the bottom of the pull request page.

After analysis, the LGTM bot will post a comment to the PR detailing newly introduced or fixed alerts, if any are found. If no new alerts have been introduced and no existing alerts fixed, no comment will be posted. 

A list of all current alerts is available on lgtm.com, which allows you to filter by language, alert severity and alert type. Suggestions for how to resolve different alert types can be found by clicking the ? icon on an alert displayed on lgtm.com.

Suppressing alerts

Whenever possible, new alerts should not be introduced by the changes in a pull request, but sometimes the LGTM analysis results in false positives, or no implementation can be found that does not result in a new LGTM alert. In these cases, and only in these cases, the LGTM alert can be suppressed, as described here. Inline, comment-based suppression is the preferred method, since it allows the user to suppress only the specified alert, and only on the line on which the comment appears.

response.addCookie(cookie); // lgtm [java/insecure-cookie]
// The lgtm alert for the above line is suppressed as a false positive, since we are simply
// using the security setting from the context's cookie config

Annotation-based suppression (only currently available for Java code) should only be used in cases where inline comments cannot be used, due to the formatting applied by the spotless gradle plugin causing the comments to span more than one line, as suppression comments must not contain line breaks. When using annotation-based alert suppression, care should be taken not to inadvertently suppress other, valid alerts in the method to which the annotation is being added.

  @SuppressWarnings("lgtm[java/useless-null-check]")
  // The "useless null check" alert is suppressed here as the first time the constructor is called,
  // QueryService.UNDEFINED is actually null, but subsequent times it is not
  public Undefined() {
    Support.assertState(QueryService.UNDEFINED == null, "UNDEFINED constant already instantiated");
  }

A comment explaining why the alert was suppressed should also be included so that other developers understand the reason behind the decision.


  • No labels