Skip to content

Commit

Permalink
feat: add Linter workflow separating it from testing, with appropriate
Browse files Browse the repository at this point in the history
helpers
  • Loading branch information
h5law committed Jan 16, 2024
1 parent 019799d commit 2eb47ca
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 15 deletions.
84 changes: 84 additions & 0 deletions .github/workflows/linter.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# The reason the linting step has been moved to its own file is due to this
# aside in the golangci-lint action repo underneath the first code block:
# Ref: https://github.com/golangci/golangci-lint-action#how-to-use
# tl;dr: "We recommend running this action in a job separate from other jobs
# (go test, etc) because different jobs run in parallel."

Name: Linter

on:
push:
branches: ["main"]
pull_request:

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.ref_name }}
cancel-in-progress: true

# Ref: https://github.com/golangci/golangci-lint-action#comments-and-annotations
permissions:
contents: read
pull-requests: read
checks: write

jobs:
lint:
runs-on: ubuntu-latest
steps:
# Firstly setup the repo to a point where golangci-lint is able to lint
# properly without getting typeerror's from revive everywhere.
- name: install ignite
# TODO_TECHDEBT: upgrade to the latest Ignite (the latest at the moment of creating a note is 0.28). Need to downgrade to fix CI pipelines. Might be done in scope of #240.
run: |
# curl https://get.ignite.com/cli! | bash
wget https://github.com/ignite/cli/releases/download/v0.27.2/ignite_0.27.2_linux_amd64.tar.gz
tar -xzf ignite_0.27.2_linux_amd64.tar.gz
sudo mv ignite /usr/local/bin/ignite
ignite version
- uses: actions/checkout@v3
with:
fetch-depth: "0" # Per https://github.com/ignite/cli/issues/1674#issuecomment-1144619147

- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: "1.20.10"
cache: false # Per https://github.com/golangci/golangci-lint-action#how-to-use

- name: Install CI dependencies
run: make install_ci_deps

- name: Generate protobufs
run: make proto_regen

# Fetch a list of all modified files in this PR.
- name: Get Modified Files
run: git diff --name-only ${{ github.event.before }} ${{ github.sha }} > modified_files.txt

# Filter the list of files modified in this PR to exclude those that have
# ignite scaffold comments in their import block, this is because gci
# (golangci-lint's import formatter) doesn't support this kind of filtering
# and removes all comments from import blocks.
- name: Filter For Lintable Files
run: go run ./tools/scripts/scaffold_filter/main.go $(cat modified_files.txt) > files_to_lint.txt

# Run golangci-lint on the filtered files from this PR - allowing gci to
# catch any improperly formatted import blocks in this PR.
- name: Run golangci-lint (filtered for scaffold comments)
uses: golangci/golangci-lint-action@v3
with:
version: latest
args: --timeout=15m --config=.golangci.yml $(cat files_to_lint.txt)
only-new-issues: true

# Do a second lint pass without import checking (after filtered lint) to
# catcy any linter errors in files that were excluded in the first pass.
# This step disables gci checks, to check for all other linter errors,
# on every file in the PR, without filtering.
- name: Run golangci-lint (without import checking)
uses: golangci/golangci-lint-action@v3
with:
version: latest
args: --timeout=15m --config=.golangci.yml --disable gci $(cat modified_files.txt)
only-new-issues: true
3 changes: 0 additions & 3 deletions .github/workflows/run-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,5 @@ jobs:
- name: Format the repo
run: make gofumpt

- name: Run golangci-lint
run: make go_lint

- name: Test
run: make go_test
12 changes: 0 additions & 12 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,10 @@ issues:
- path: ^x/.+/genesis\.go$
linters:
- gci
- lll
- gofumpt
# Exclude cosmos-sdk module module.go files as they are generated with unused
# parameters and unchecked errors.
- path: ^x/.+/module\.go$
linters:
- gci
- lll
- gofumpt
- revive
- errcheck
# Exclude cosmos-sdk module tx.go files as they are generated with unused
Expand All @@ -67,22 +62,18 @@ issues:
linters:
- gci
- lll
- gofumpt
- unused
# Exclude simulation code as it's generated with lots of unused parameters.
- path: .*/simulation/.*|_simulation\.go$
linters:
- gci
- lll
- gofumpt
- revive
# Exclude cosmos-sdk module codec files as they are scaffolded with a unused
# parameters and a comment used by ignite CLI.
- path: ^x/.+/codec.go$
linters:
- gci
- lll
- gofumpt
- revive
- path: _test\.go$
linters:
Expand All @@ -92,9 +83,6 @@ issues:
- path: errors\.go$
linters:
- lll
- path: ./app/app.go
linters:
- gci
# TODO_IMPROVE: see https://golangci-lint.run/usage/configuration/#issues-configuration
#new: true,
#fix: true,
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ localnet_regenesis: ## Regenerate the localnet genesis file
.PHONY: go_lint
go_lint: ## Run all go linters
golangci-lint run

.PHONY: gci
gci: check_gci ## Run gci (import ordering) on all applicable files, this writes changes in place
go run ./tools/scripts/gci
Expand Down
109 changes: 109 additions & 0 deletions tools/scripts/scaffold_filter/filters/filters.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package filters

import (
"bufio"
"bytes"
"os"
"strings"
)

const igniteScaffoldComment = "// this line is used by starport scaffolding"

var (
importStart = []byte("import (")
importEnd = []byte(")")
)

// FilterFn is a function that returns true if the given path matches the
// filter's criteria.
type FilterFn func(path string) (bool, error)

// ImportBlockContainsScaffoldComment matches import blocks containing the
// `// this line is used by starport scaffolding` comment.
func ImportBlockContainsScaffoldComment(path string) (bool, error) {
return containsImportScaffoldComment(path)
}

// ContentMatchesEmptyImportScaffold matches files that can't be goimport'd due
// to ignite incompatibility.
func ContentMatchesEmptyImportScaffold(path string) (bool, error) {
return containsEmptyImportScaffold(path)
}

// containsEmptyImportScaffold checks if the go file at goSrcPath contains an
// import statement like the following:
//
// import (
// // this line is used by starport scaffolding ...
// )
func containsEmptyImportScaffold(goSrcPath string) (isEmptyImport bool, _ error) {
file, err := os.Open(goSrcPath)
if err != nil {
return false, err
}
defer file.Close()

scanner := bufio.NewScanner(file)
scanner.Split(importBlockSplit)

for scanner.Scan() {
trimmedImportBlock := strings.Trim(scanner.Text(), "\n\t")
if strings.HasPrefix(trimmedImportBlock, igniteScaffoldComment) {
return true, nil
}
}

if scanner.Err() != nil {
return false, scanner.Err()
}

return false, nil
}

// containsImportScaffoldComment checks if the go file at goSrcPath contains a
// comment in the import block like the following:
//
// // this line is used by starport scaffolding ...
func containsImportScaffoldComment(goSrcPath string) (containsComment bool, _ error) {
file, err := os.Open(goSrcPath)
if err != nil {
return false, err
}
defer file.Close()

scanner := bufio.NewScanner(file)
scanner.Split(importBlockSplit)

for scanner.Scan() {
trimmedImportBlock := strings.Trim(scanner.Text(), "\n\t")
if strings.Contains(trimmedImportBlock, igniteScaffoldComment) {
return true, nil
}
}

if scanner.Err() != nil {
return false, scanner.Err()
}

return false, nil
}

// importBlockSplit is a split function intended to be used with bufio.Scanner
// to extract the contents of a multi-line go import block.
func importBlockSplit(data []byte, _ bool) (advance int, token []byte, err error) {
// Search for the beginning of the import block
startIdx := bytes.Index(data, importStart)
if startIdx == -1 {
return 0, nil, nil
}

// Search for the end of the import block from the start index
endIdx := bytes.Index(data[startIdx:], importEnd)
if endIdx == -1 {
return 0, nil, nil
}

// Return the entire import block, including "import (" and ")"
importBlock := data[startIdx+len(importStart) : startIdx-len(importEnd)+endIdx+1]
return startIdx + endIdx + 1, importBlock, nil
}
48 changes: 48 additions & 0 deletions tools/scripts/scaffold_filter/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package main

import (
"fmt"
"os"

"github.com/pokt-network/poktroll/tools/scripts/gci/filters"
)

var defaultExcludeFilters = []filters.FilterFn{
filters.ContentMatchesEmptyImportScaffold,
filters.ImportBlockContainsScaffoldComment,
}

func main() {
if len(os.Args) < 2 {
fmt.Printf("Usage: %s <path>...\n", os.Args[0])
os.Exit(1)
}

var filesToProcess []string
// For each file passed as an argument check whether to exclude it
// according to the filters, from a golangci-lint check. This is
// purely because gci (the golanci-lint linter for imports) doesn't
// support this type of filtering natively.
for _, path := range os.Args[1:] {
for _, excludeFilter := range defaultExcludeFilters {
shouldExclude, err := excludeFilter(path)
if err != nil {
fmt.Printf("Error processing file %s: %s\n", path, err)
continue
}
if !shouldExclude {
filesToProcess = append(filesToProcess, path)
}
}
}

// Print all files to be processed by golangci-lint.
// This is so gci doesn't remove the scaffold comments in
// the import block for files scaffolded by ignite.
for _, file := range filesToProcess {
// We print the file names as this is a helper used in the
// testing workflow and the output of this file is appended
// to a file from which golangci-lint will be run on, in CI.
fmt.Println(file)
}
}

0 comments on commit 2eb47ca

Please sign in to comment.