Skip to content

Commit

Permalink
doc: Addiding Documentation for Linting Rules (#81)
Browse files Browse the repository at this point in the history
# Description

- Add RFC documentation for each lint rules
- Add RFC templates
- Updated README regarding adding lint rules to the README
  • Loading branch information
notJoon authored Sep 30, 2024
1 parent 422dcdb commit be21dd1
Show file tree
Hide file tree
Showing 11 changed files with 705 additions and 24 deletions.
6 changes: 4 additions & 2 deletions .github/labeler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,13 @@ jobs:
repo-token: "${{ secrets.GITHUB_TOKEN }}"
configuration-path: .github/labeler.yml

---

A-Action:
- '**/*.yml'

A-documentation:
- '**/*.md'
- 'internal/lints/README.md'
- '!docs/rfc/**/*'

A-lint:
- 'internal/lints/**/*'
Expand All @@ -41,3 +40,6 @@ T-CLI:
L-test:
- '**/*_test.go'
- 'testdata/**/*'

RFC:
- 'docs/rfc/**/*'
62 changes: 40 additions & 22 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,36 +62,48 @@ To check the current directory, run:
tlin .
```

## Adding Custom Lint Rules
## Adding Gno-Specific Lint Rules

tlin allows addition of custom lint rules beyond the default golangci-lint rules. To add a new lint rule, follow these steps:
Our linter allows addition of custom lint rules beyond the default golangci-lint rules. To add a new lint rule, follow these steps:

> ⚠️ Must update relevant tests if you have added a new rule or formatter.
1. Implement the `LintRule` interface for your new rule:
1. Create an RFC (Request for Comments) document for your proposed lint rule:
- Describe the purpose and motivation for the new rule. You can find template in [RFC](./docs/rfc/template.md)
- Provide examples of code that the rule will flag.
- Explain any potential edge cases or considerations.
- Outline the proposed implementation approach.

```go
type NewRule struct{}
2. Open a new issue in the tlin repository and attach your RFC document.

func (r *NewRule) Check(filename string, node *ast.File) ([]types.Issue, error) {
// Implement your lint rule logic here
// return a slice of Issues and any error encountered
}
```
3. Wait for community feedback and maintainer approval. Be prepared to iterate on your RFC based on feedback.

2. Register your new rule in the `registerDefaultRules` method of the `Engine` struct in `internal/engine.go`:
4. Once the RFC is approved, proceed with the implementation:

```go
func (e *Engine) registerDefaultRules() {
e.rules = append(e.rules,
&GolangciLintRule{},
// ...
&NewRule{}, // Add your new rule here
)
}
```
a. Implement the `LintRule` interface for your new rule:

```go
type NewRule struct{}

3. (Optional) if your rule requires special formatting, create a new formatter in the `formatter` package:
func (r *NewRule) Check(filename string, node *ast.File) ([]types.Issue, error) {
// Implement your lint rule logic here
// return a slice of Issues and any error encountered
}
```

b. Register your new rule in the `registerDefaultRules` method of the `Engine` struct in `internal/engine.go`:

```go
func (e *Engine) registerDefaultRules() {
e.rules = append(e.rules,
&GolangciLintRule{},
// ...
&NewRule{}, // Add your new rule here
)
}
```

5. (Optional) If your rule requires special formatting, create a new formatter in the `formatter` package:

a. Create a new file (e.g., `formatter/new_rule.go`).

Expand Down Expand Up @@ -122,7 +134,13 @@ func (e *Engine) registerDefaultRules() {
}
```

By following these steps, you can add new lint rules and ensure they are properly formatted when displayed in the CLI.
6. Add comprehensive tests for your new rule and formatter.

7. Update the documentation to include information about the new rule.

8. Submit a pull request with your implementation, tests, and documentation updates.

By following these steps, you can propose, discuss, and add new lint rules in a structured manner, ensuring they are properly integrated into the tlin project.

## Available Flags

Expand Down
112 changes: 112 additions & 0 deletions docs/rfc/defer-checks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# RFC: Defer Checks

## Summary

This set of lint rules checks for common issues and best practices related to the use of `defer` statements in Go code. It includes checks for deferring panics, nil functions, returns in deferred functions, and defers in loops.

## Motivation

The `defer` statement can be misused in ways that lead to unexpected behavior or performance issues. These lint rules aim to catch common mistakes and anti-patterns related to `defer`, preventing potential runtime errors.

## Proposed Implementation

1. Deferring panics
2. Deferring potentially nil functions
3. Using return statements in deferred functions
4. Using defer inside loops

### Rule Details

#### 1. Defer Panic

- **Rule ID**: defer-panic
- **Severity**: warning
- **Category**: best practices
- **Auto-fixable**: No
- **Description**: Detects when `panic` is called inside a deferred function.

#### 2. Defer Nil Function

- **Rule ID**: defer-nil-func
- **Severity**: error
- **Category**: bug prevention
- **Auto-fixable**: No
- **Description**: Detects when a potentially nil function is deferred.

#### 3. Return in Defer

- **Rule ID**: return-in-defer
- **Severity**: warning
- **Category**: best practices
- **Auto-fixable**: No
- **Description**: Detects when a return statement is used inside a deferred function.

#### 4. Defer in Loop

- **Rule ID**: defer-in-loop
- **Severity**: warning
- **Category**: performance
- **Auto-fixable**: No
- **Description**: Detects when defer is used inside a loop.

### Code Examples

#### Incorrect:

```go
func example() {
defer panic("This is bad") // defer-panic

var f func()
defer f() // defer-nil-func

defer func() {
return // return-in-defer
}()

for i := 0; i < 10; i++ {
defer fmt.Println(i) // defer-in-loop
}
}
```

#### Correct:

```go
func example() {
defer func() {
if err := recover(); err != nil {
// Handle the panic
}
}()

f := func() { /* do something */ }
defer f()

defer func() {
// Perform cleanup without using return
}()

cleanup := func(i int) {
fmt.Println(i)
}
for i := 0; i < 10; i++ {
i := i // Create a new variable to capture the loop variable
defer cleanup(i)
}
}
```

## Alternatives

1. Runtime checks: Some of these issues could be caught at runtime, but this would not prevent the errors from occurring in production.

## Implementation Impact

- Positive: Improved code quality and prevention of common `defer`-related bugs.

## Open Questions

1. Should we consider adding severity levels for each rule that can be configured by users?
2. Are there any additional `defer`-related checks we should consider implementing?
3. Should we provide auto-fix for any of these rules?
90 changes: 90 additions & 0 deletions docs/rfc/early-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# RFC: Early Return Opportunities

## Summary

This lint rule detects if-else chains that can be simplified using early returns. It aims to improve code readability by flattening unnecessary nested structures.

## Motivation

Deeply nested if-else chains can make code harder to read and maintain. By identifying opportunities for early returns, we can simplify the code structure, making it more linear and easier to follow. This not only improves readability but can also reduce the cognitive load on developers working with the code.

## Proposed Implementation

The implementation consists of a main function `DetectEarlyReturnOpportunities` and several helper functions. The core logic includes:

1. Analyzing if-else chains in the AST.
2. Identifying opportunities for early returns.
3. Generating suggestions for code improvement.
4. Providing detailed issue reports.

### Rule Details

- **Rule ID**: early-return
- **Severity**: warning
- **Category**: code style
- **Auto-fixable**: Yes[^1]
- **Description**: Detects if-else chains that can be simplified using early returns.

### Key Components

1. `analyzeIfElseChain`: Builds a representation of the if-else chain.
2. `canUseEarlyReturn`: Determines if an early return can be applied.
3. `RemoveUnnecessaryElse`: Generates an improved version of the code.
4. `extractSnippet`: Extracts the relevant code snippet for analysis.
5. `generateEarlyReturnSuggestion`: Creates a suggestion for code improvement.

### Code Examples

#### Before:

```go
if condition1 {
// some code
return result1
} else {
if condition2 {
// some more code
return result2
} else {
// final code
return result3
}
}
```

#### After:

```go
if condition1 {
// some code
return result1
}
if condition2 {
// some more code
return result2
}
// final code
return result3
```

## Alternatives

1. Manual refactoring: Relying on developers to identify and refactor these patterns manually.

## Implementation Impact

- Positive: Improved code readability and maintainability.
- Negative: May require significant changes to existing codebases if widely applied.

## Open Questions

1. Should we consider the complexity of the conditions when suggesting early returns?
2. How should we handle cases where the else block contains multiple statements?
3. Should we provide a configuration option to set a maximum nesting level for applying this rule?
4. How do we ensure that the meaning of the code is preserved when applying early returns, especially in the presence of defer statements or other Go-specific constructs?

## References

- [linux coding style: indentation](https://github.com/torvalds/linux/blob/master/Documentation/process/coding-style.rst#1-indentation)

[^1]: This lint rule is auto-fixable, but still contains some edge cases that are not handled that need to be handled.
54 changes: 54 additions & 0 deletions docs/rfc/emit-event.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# RFC: Emit Format

## Summary

This lint rule ensures that `std.Emit` calls are properly formatted for better readability, especially when they contain multiple arguments.

## Motivation

The `std.Emit` function is commonly used for logging and event emission in Go programs. When these calls contain multiple key-value pairs, they can become difficult to read if not properly formatted. This rule aims to improve code readability and maintainability by enforcing a consistent formatting style for `std.Emit` calls.

## Proposed Implementation

The rule will check for `std.Emit` calls and suggest a formatted version if the call is not properly structured. The formatting guidelines are:

1. The `std.Emit` call should be multi-line if it has more than 3 arguments.
2. The event type (first argument) should be on its own line.
3. Each key-value pair should be on its own line.
4. The closing parenthesis should be on a new line.

### Rule Details

- **Rule ID**: emit-format
- **Severity**: warning
- **Category**: Style
- **Auto-fixable**: Yes

### Code Examples

#### Incorrect:

```go
std.Emit(
"OwnershipChange",
"newOwner", newOwner.String(),
"oldOwner",
oldOwner.String(),
"anotherOwner", anotherOwner.String(),
)
```

#### Correct:

```go
std.Emit(
"OwnershipChange", // event type
"newOwner", newOwner.String(), // key-value pair
"oldOwner", oldOwner.String(), // key-value pair
"anotherOwner", anotherOwner.String(), // key-value pair
)
```

## References

- [Effective gno](https://docs.gno.land/concepts/effective-gno#emit-gno-events-to-make-life-off-chain-easier)
Loading

0 comments on commit be21dd1

Please sign in to comment.