Skip to content

Changed everything across the repo #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Changed everything across the repo #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added .DS_Store
View file
Open in desktop
Binary file not shown.
70 changes: 52 additions & 18 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,74 @@
PROJECT_ROOT = $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
export GOBIN = $(PROJECT_ROOT)/bin

GOLANGCI_LINT_VERSION := $(shell golangci-lint --version 2>/dev/null)
GOLANGCI_LINT_VERSION := $(shell $(GOBIN)/golangci-lint version --format short 2>/dev/null)
REQUIRED_GOLANGCI_LINT_VERSION := $(shell cat .golangci.version)

# Directories containing independent Go modules.
MODULE_DIRS = . ./tools

.PHONY: all
all: build lint test
all: build lint test integration-test

.PHONY: clean
clean:
@rm -rf $(GOBIN)

.PHONY: build
build:
go install go.uber.org/nilaway/cmd/nilaway

.PHONY: test
test:
go test -v -race ./...
@$(foreach mod,$(MODULE_DIRS),(cd $(mod) && go test -race ./...) &&) true

.PHONY: cover
cover:
go test -v -race -coverprofile=cover.out -coverpkg=./... -v ./...
go tool cover -html=cover.out -o cover.html
@$(foreach mod,$(MODULE_DIRS), ( \
cd $(mod) && \
go test -race -coverprofile=cover.out -coverpkg=./... ./... \
&& go tool cover -html=cover.out -o cover.html) &&) true

.PHONY: golden-test
golden-test:
@cd tools && go install go.uber.org/nilaway/tools/cmd/golden-test
@$(GOBIN)/golden-test $(ARGS)

.PHONY: integration-test
integration-test:
@cd tools && go install go.uber.org/nilaway/tools/cmd/integration-test
@$(GOBIN)/integration-test

.PHONY: lint
lint: golangci-lint tidy-lint
lint: golangci-lint nilaway-lint tidy-lint

# Install golangci-lint with the required version in GOBIN if it is not already installed.
.PHONY: install-golangci-lint
install-golangci-lint:
ifneq ($(GOLANGCI_LINT_VERSION),$(REQUIRED_GOLANGCI_LINT_VERSION))
@echo "[lint] installing golangci-lint v$(REQUIRED_GOLANGCI_LINT_VERSION) since current version is \"$(GOLANGCI_LINT_VERSION)\""
@curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOBIN) v$(REQUIRED_GOLANGCI_LINT_VERSION)
endif

.PHONY: golangci-lint
golangci-lint:
ifdef GOLANGCI_LINT_VERSION
@echo "[lint] $(GOLANGCI_LINT_VERSION)"
else
$(error "golangci-lint not found, please install it from https://golangci-lint.run/usage/install/#local-installation")
endif
@echo "[lint] golangci-lint run"
@golangci-lint run
golangci-lint: install-golangci-lint
@echo "[lint] $(shell $(GOBIN)/golangci-lint version)"
@$(foreach mod,$(MODULE_DIRS), \
(cd $(mod) && \
echo "[lint] golangci-lint: $(mod)" && \
$(GOBIN)/golangci-lint run --path-prefix $(mod)) &&) true

.PHONY: tidy-lint
tidy-lint:
@echo "[lint] go mod tidy"
@go mod tidy && \
git diff --exit-code -- go.mod go.sum || \
(echo "'go mod tidy' changed files" && false)
@$(foreach mod,$(MODULE_DIRS), \
(cd $(mod) && \
echo "[lint] mod tidy: $(mod)" && \
go mod tidy && \
git diff --exit-code -- go.mod go.sum) &&) true

.PHONY: nilaway-lint
nilaway-lint: build
@$(foreach mod,$(MODULE_DIRS), \
(cd $(mod) && \
echo "[lint] nilaway linting itself: $(mod)" && \
$(GOBIN)/nilaway -include-pkgs="go.uber.org/nilaway" ./...) &&) true
195 changes: 181 additions & 14 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
# NilAway

[![GoDoc][doc-img]][doc] [![Build Status][ci-img]][ci] [![Coverage Status][cov-img]][cov]

> [!WARNING]
> NilAway is currently under active development: false positives and breaking changes can happen.
> We highly appreciate any feedback and contributions!

NilAway is a static analysis tool that seeks to help developers avoid nil panics in production by catching them at
compile time rather than runtime. NilAway is similar to the standard
[nilness analyzer](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/nilness), however, it employs much more
sophisticated and powerful static analysis techniques to track nil flows within a package as well _across_ packages, and
report errors providing users with the nilness flows for easier debugging.
report errors providing users with the nilness flows for easier debugging.
Comment on lines 11 to +13
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Sentence on line 13 feels a bit awkward: consider rephrasing for clarity (e.g. 'and reports errors by displaying the nilness flow for easier debugging').


NilAway enjoys three key properties that make it stand out:

Expand All @@ -19,26 +25,154 @@ optimizations to further reduce its footprint.
nil panics we have observed in production, allowing NilAway to maintain a good balance between usefulness and build-time
overhead.

## Installation
:star2: For more detailed technical discussion, please check our [Wiki][wiki], [Engineering Blog][blog], and paper (WIP).

## Running NilAway

NilAway is implemented using the standard [go/analysis](https://pkg.go.dev/golang.org/x/tools/go/analysis) framework,
making it easy to integrate with existing analyzer drivers (e.g., [golangci-lint](https://github.com/golangci/golangci-lint),
[nogo](https://github.com/bazelbuild/rules_go/blob/master/go/nogo.rst), or
[running as a standalone checker](https://pkg.go.dev/golang.org/x/tools/go/analysis/singlechecker)). Here, we list the
instructions for running NilAway as a standalone checker. More integration supports will be added soon.
NilAway is implemented using the standard [go/analysis][go-analysis], making it easy to integrate with existing analyzer
drivers (i.e., [golangci-lint][golangci-lint], [nogo][nogo], or [running as a standalone checker][singlechecker]).

> [!IMPORTANT]
> By default, NilAway analyzes _all_ Go code, including the standard libraries and dependencies. This helps NilAway
> better understand the code form dependencies and reduce its false negatives. However, this would also incur a
> significant performance cost (only once for drivers with modular support) and increase the number of non-actionable
> errors in dependencies, for large Go projects with a lot of dependencies.
>
> We highly recommend using the [include-pkgs][include-pkgs-flag] flag to narrow down the analysis to your project's
> code exclusively. This directs NilAway to skip analyzing dependencies (e.g., third-party libraries), allowing you to
> focus solely on potential nil panics reported by NilAway in your first-party code!

### Standalone Checker

> [!IMPORTANT]
> Due to the sophistication of the analyses that NilAway does, NilAway caches its findings about a
> particular package via the [Fact Mechanism][fact-mechanism] from the [go/analysis][go-analysis]
> framework. Therefore, it is _highly_ recommended to leverage a driver that supports modular
> analysis (i.e., bazel/nogo or golangci-lint, but _not_ the standalone checker since it stores all
> facts in memory) for better performance on large projects. The standalone checker is provided
> more for evaluation purposes since it is easy to get started.

Install the binary from source by running:
```shell
go install go.uber.org/nilaway/cmd/nilaway@latest
```

Then, run the linter by:
```shell
nilaway ./...
nilaway -include-pkgs="<YOUR_PKG_PREFIX>,<YOUR_PKG_PREFIX_2>" ./...
```

> [!TIP]
> Disable the `pretty-print` flag when output as JSON:
> ```shell
> nilaway -json -pretty-print=false -include-pkgs="<YOUR_PKG_PREFIX>,<YOUR_PKG_PREFIX_2>" ./...
> ```


### golangci-lint (>= v1.57.0)

NilAway, in its current form, can report false positives. This unfortunately hinders its immediate
merging in [golangci-lint][golangci-lint] and be offered as a linter (see [PR#4045][pr-4045]).
Therefore, you need to build NilAway as a plugin to golangci-lint to be executed as a private
linter. There are two plugin systems in golangci-lint, and it is much easier to use the
Comment on lines +74 to +77
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The phrase 'merging in golangci-lint and be offered as a linter' may be grammatically improved. Consider rewording for clarity.

[Module Plugin System][golangci-lint-module-plugin] (introduced since v1.57.0), and it is the only
supported approach to run NilAway in golangci-lint.

(1) Create a `.custom-gcl.yml` file at the root of the repository if you have not done so, add the
following content:

```yaml
# This has to be >= v1.57.0 for module plugin system support.
version: v1.57.0
plugins:
- module: "go.uber.org/nilaway"
import: "go.uber.org/nilaway/cmd/gclplugin"
version: latest # Or a fixed version for reproducible builds.
```

(2) Add NilAway to the linter configuration file `.golangci.yaml`:

```yaml
linters-settings:
custom:
nilaway:
type: "module"
description: Static analysis tool to detect potential nil panics in Go code.
settings:
# Settings must be a "map from string to string" to mimic command line flags: the keys are
# flag names and the values are the values to the particular flags.
include-pkgs: "<YOUR_PACKAGE_PREFIXES>"
# NilAway can be referred to as `nilaway` just like any other golangci-lint analyzers in other
# parts of the configuration file.
```

(3) Build a custom golangci-lint binary with NilAway included:

```shell
# Note that your `golangci-lint` to bootstrap the custom binary must also be version >= v1.57.0.
$ golangci-lint custom
```

By default, the custom binary will be built at `.` with the name `custom-gcl`, which can be further
customized in `.custom-gcl.yml` file (see [Module Plugin System][golangci-lint-module-plugin] for
instructions).

> [!TIP]
> Cache the custom binary to avoid having to build it again to save resources, you can use the
> hash of the `.custom-gcl.yml` file as the cache key if you are using a fixed version of NilAway.
> If you are using `latest` as NilAway version, you can append the date of build to the cache key
> to force cache expiration after certain time period.

(4) Run the custom binary instead of `golangci-lint`:

```shell
# Arguments are the same as `golangci-lint`.
$ ./custom-gcl run ./...
```

### Bazel/nogo

Running with bazel/nogo requires slightly more efforts. First follow the instructions from [rules_go][rules-go],
[gazelle][gazelle], and [nogo][nogo] to set up your Go project such that it can be built with bazel/nogo with no or
default set of linters configured. Then,

(1) Add `import _ "go.uber.org/nilaway"` to your `tools.go` file (or other file that you use for configuring tool
dependencies, see [How can I track tool dependencies for a module?][track-tool-dependencies] from Go Modules
documentation) to avoid `go mod tidy` from removing NilAway as a tool dependency.

(2) Run the following commands to add NilAway as a tool dependency to your project:
```bash
# Get NilAway as a dependency, as well as getting its transitive dependencies in go.mod file.
$ go get go.uber.org/nilaway@latest
# This should not remove NilAway as a dependency in your go.mod file.
$ go mod tidy
# Run gazelle to sync dependencies from go.mod to WORKSPACE file.
$ bazel run //:gazelle -- update-repos -from_file=go.mod
```

(3) Add NilAway to nogo configurations (usually in top-level `BUILD.bazel` file):

```diff
nogo(
name = "my_nogo",
visibility = ["//visibility:public"], # must have public visibility
deps = [
+++ "@org_uber_go_nilaway//:go_default_library",
],
config = "config.json",
)
```

(4) Run bazel build to see NilAway working (any nogo error will stop the bazel build, you can use the `--keep_going`
flag to request bazel to build as much as possible):

```bash
$ bazel build --keep_going //...
```

(5) See [nogo documentation][nogo-configure-analyzers] on how to pass a configuration JSON to the nogo driver, and see
our [wiki page][nogo-configure-nilaway] on how to pass configurations to NilAway.

## Code Examples

Let's look at a few examples to see how NilAway can help prevent nil panics.
Expand All @@ -57,7 +191,8 @@ panic may occur if `someCondition` is false. NilAway is able to catch this poten
error showing this nilness flow:

```
go.uber.org/example.go:12:9: error: Value read from a variable that was never assigned to (definitely nilable) and is passed to a field access at go.uber.org/example.go:12:9 (must be nonnil)
go.uber.org/example.go:12:9: error: Potential nil panic detected. Observed nil flow from source to dereference point:
- go.uber.org/example.go:12:9: unassigned variable `p` accessed field `f`
```

If we guard this dereference with a nilness check (`if p != nil`), the error goes away.
Expand All @@ -79,18 +214,26 @@ whenever `bar` is called. NilAway is able to catch this potential nil flow and r
the nilness flow across function boundaries:

```
go.uber.org/example.go:19:6: error: Annotation on Result 0 of Function foo overconstrained:
Must be NILABLE because it describes the value returned from the function `foo` in position 0 at go.uber.org/example.go:20:14, and that value is literal nil at go.uber.org/example.go:20:14, where it is NILABLE
AND
Must be NONNIL because it describes the value returned as result 0 from the method `foo`, and that value is dereferenced at go.uber.org/example.go:23:13, where it must be NONNIL
go.uber.org/example.go:23:13: error: Potential nil panic detected. Observed nil flow from source to dereference point:
- go.uber.org/example.go:20:14: literal `nil` returned from `foo()` in position 0
- go.uber.org/example.go:23:13: result 0 of `foo()` dereferenced
```

Note that in the above example, `foo` does not necessarily have to reside in the same package as `bar`. NilAway is able
to track nil flows across packages as well. Moreover, NilAway handles Go-specific language constructs such as receivers,
interfaces, type assertions, type switches, and more. For more detailed discussion, please check our paper.
interfaces, type assertions, type switches, and more.

## Configurations

We expose a set of flags via the standard flag passing mechanism in [go/analysis](https://pkg.go.dev/golang.org/x/tools/go/analysis).
Please check [wiki/Configuration](https://github.com/uber-go/nilaway/wiki/Configuration) to see the available flags and
how to pass them using different linter drivers.

## Support

We follow the same [version support policy](https://go.dev/doc/devel/release#policy) as the [Go](https://golang.org/)
project: we support and test the last two major versions of Go.

Please feel free to [open a GitHub issue](https://github.com/uber-go/nilaway/issues) if you have any questions, bug
reports, and feature requests.

Expand All @@ -102,3 +245,27 @@ our [Uber Contributor License Agreement](https://cla-assistant.io/uber-go/nilawa
## License

This project is copyright 2023 Uber Technologies, Inc., and licensed under Apache 2.0.

[go-analysis]: https://pkg.go.dev/golang.org/x/tools/go/analysis
[golangci-lint]: https://github.com/golangci/golangci-lint
[golangci-lint-module-plugin]: https://golangci-lint.run/plugins/module-plugins/
[singlechecker]: https://pkg.go.dev/golang.org/x/tools/go/analysis/singlechecker
[nogo]: https://github.com/bazelbuild/rules_go/blob/master/go/nogo.rst
[doc-img]: https://pkg.go.dev/badge/go.uber.org/nilaway.svg
[doc]: https://pkg.go.dev/go.uber.org/nilaway
[ci-img]: https://github.com/uber-go/nilaway/actions/workflows/ci.yml/badge.svg
[ci]: https://github.com/uber-go/nilaway/actions/workflows/ci.yml
[cov-img]: https://codecov.io/gh/uber-go/nilaway/branch/main/graph/badge.svg
[cov]: https://codecov.io/gh/uber-go/nilaway
[wiki]: https://github.com/uber-go/nilaway/wiki
[blog]: https://www.uber.com/blog/nilaway-practical-nil-panic-detection-for-go/
[fact-mechanism]: https://pkg.go.dev/golang.org/x/tools/go/analysis#hdr-Modular_analysis_with_Facts
[include-pkgs-flag]: https://github.com/uber-go/nilaway/wiki/Configuration#include-pkgs
[pr-4045]: https://github.com/golangci/golangci-lint/issues/4045
[nilaway-as-a-plugin]: https://golangci-lint.run/contributing/new-linters/#how-to-add-a-private-linter-to-golangci-lint
[rules-go]: https://github.com/bazelbuild/rules_go
[gazelle]: https://github.com/bazelbuild/bazel-gazelle
[track-tool-dependencies]: https://go.dev/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
[nogo-configure-analyzers]: https://github.com/bazelbuild/rules_go/blob/master/go/nogo.rst#id14
[nogo-configure-nilaway]: https://github.com/uber-go/nilaway/wiki/Configuration#nogo
[nogo-instructions]: https://github.com/uber-go/nilaway?tab=readme-ov-file#bazelnogo
Loading