From be21dd15c7aa58b587f01b7b165970a14db7af23 Mon Sep 17 00:00:00 2001 From: Lee ByeongJun Date: Mon, 30 Sep 2024 18:03:50 +0900 Subject: [PATCH] doc: Addiding Documentation for Linting Rules (#81) # Description - Add RFC documentation for each lint rules - Add RFC templates - Updated README regarding adding lint rules to the README --- .github/labeler.yml | 6 +- README.md | 62 ++++++++----- docs/rfc/defer-checks.md | 112 ++++++++++++++++++++++++ docs/rfc/early-return.md | 90 +++++++++++++++++++ docs/rfc/emit-event.md | 54 ++++++++++++ docs/rfc/mod-tidy.md | 66 ++++++++++++++ docs/rfc/repeated-regex-compilation.md | 72 +++++++++++++++ docs/rfc/template.md | 54 ++++++++++++ docs/rfc/unnecesary-slice-length.md | 53 +++++++++++ docs/rfc/unnecessary-break.md | 78 +++++++++++++++++ docs/rfc/unnecessary-type-conversion.md | 82 +++++++++++++++++ 11 files changed, 705 insertions(+), 24 deletions(-) create mode 100644 docs/rfc/defer-checks.md create mode 100644 docs/rfc/early-return.md create mode 100644 docs/rfc/emit-event.md create mode 100644 docs/rfc/mod-tidy.md create mode 100644 docs/rfc/repeated-regex-compilation.md create mode 100644 docs/rfc/template.md create mode 100644 docs/rfc/unnecesary-slice-length.md create mode 100644 docs/rfc/unnecessary-break.md create mode 100644 docs/rfc/unnecessary-type-conversion.md diff --git a/.github/labeler.yml b/.github/labeler.yml index 40fc7ec..095ff12 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -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/**/*' @@ -41,3 +40,6 @@ T-CLI: L-test: - '**/*_test.go' - 'testdata/**/*' + +RFC: + - 'docs/rfc/**/*' diff --git a/README.md b/README.md index ef0acaf..2d96fdf 100644 --- a/README.md +++ b/README.md @@ -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`). @@ -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 diff --git a/docs/rfc/defer-checks.md b/docs/rfc/defer-checks.md new file mode 100644 index 0000000..4320116 --- /dev/null +++ b/docs/rfc/defer-checks.md @@ -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? diff --git a/docs/rfc/early-return.md b/docs/rfc/early-return.md new file mode 100644 index 0000000..b91b8cb --- /dev/null +++ b/docs/rfc/early-return.md @@ -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. diff --git a/docs/rfc/emit-event.md b/docs/rfc/emit-event.md new file mode 100644 index 0000000..e6ba6f4 --- /dev/null +++ b/docs/rfc/emit-event.md @@ -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) \ No newline at end of file diff --git a/docs/rfc/mod-tidy.md b/docs/rfc/mod-tidy.md new file mode 100644 index 0000000..07ffb17 --- /dev/null +++ b/docs/rfc/mod-tidy.md @@ -0,0 +1,66 @@ +# RFC: Gno Mod Tidy + +## Summary + +This lint rule checks for inconsistencies between the packages imported in Gno source files and those declared in the `gno.mod` file. It helps ensure that the `gno.mod` file accurately reflects the project's dependencies. + +## Motivation + +In Gno projects, the `gno.mod` file serves a similar purpose to Go's `go.mod` file, declaring the project's dependencies. Keeping this file in sync with the actual imports used in the source code is crucial for proper dependency management. This lint rule aims to catch discrepancies early, promoting better dependency hygiene and preventing potential build or runtime issues. + +## Proposed Implementation + +The implementation consists of a function `DetectMissingModPackage` that performs the following checks: + +1. Extracts imports from the Gno source files. +2. Extracts declared packages from the `gno.mod` file. +3. Compares the two sets to identify: + a. Packages declared in `gno.mod` but not imported in the source. + b. Packages imported in the source but not declared in `gno.mod`. + +### Rule Details + +- **Rule ID**: gno-mod-tidy +- **Severity**: warning +- **Category**: dependency management +- **Auto-fixable**: No (but suggests running `gno mod tidy`) +- **Description**: Detects inconsistencies between imported packages and those declared in `gno.mod`. + +### Code Examples + +#### Incorrect: + +```go +// bar.gno +package bar + +import ( + "gno.land/p/demo/avl" +) + +// gno.mod +module example.com/mymodule + +gno.land/p/demo/avl v0.0.0-latest +gno.land/p/demo/foo v0.0.0-latest // Unused package +``` + +#### Correct: + +```go +// bar.gno +package bar + +import ( + "gno.land/p/demo/avl" +) + +// gno.mod +module example.com/mymodule + +gno.land/p/demo/avl v0.0.0-latest +``` + +## Implementation Impact + +- Positive: Improved dependency management and prevention of potential runtime errors due to missing or unnecessary dependencies. diff --git a/docs/rfc/repeated-regex-compilation.md b/docs/rfc/repeated-regex-compilation.md new file mode 100644 index 0000000..b36121e --- /dev/null +++ b/docs/rfc/repeated-regex-compilation.md @@ -0,0 +1,72 @@ +# RFC: Repeated Regex Compilation + +## Summary + +This lint rule detects repeated compilation of the same regex pattern within a function, which can lead to performance issues. + +## Motivation + +Compiling regex is a relatively expensive operation. Repeatedly compiling the same pattern, especially within a single function, can unnecessarily impact performance. + +By identifying these instances, we can optimize their code by compiling regex patterns once and reusing the compiled version. + +## Proposed Implementation + +The implementation consists of an `analysis.Analyzer` named `RepeatedRegexCompilationAnalyzer` and several supporting functions. The core logic includes: + +1. Loading the package using `x/tools/go/packages`. +2. Running the analyzer on the loaded package. +3. Inspecting each function declaration for repeated regex compilations. +4. Reporting issues when repeated compilations are found. + +### Rule Details + +- **Rule ID**: repeatedregexcompilation +- **Severity**: warning +- **Category**: performance +- **Auto-fixable**: No +- **Description**: Checks for repeated compilation of the same regex pattern within a function. + +### Use of x/tools/go/packages + +The `x/tools/go/packages` package is used to load and parse Go packages. It provides a way to access the syntax tree, type information, and other details of Go packages. In this implementation, it's used with the following configuration: + +```go +cfg := &packages.Config{ + Mode: packages.NeedFiles | packages.NeedSyntax | packages.NeedTypesInfo | packages.NeedTypes, + Tests: false, +} +``` + +This configuration loads the necessary information for analysis, including files, syntax trees, and type information, but excludes test files. + +The package is crucial for this lint rule as it allows us to analyze the entire package context, which is necessary for accurate detection of issues across multiple files and with respect to imported packages. + +### Code Examples + +#### Incorrect: + +```go +func processData(data []string) { + for _, d := range data { + r, _ := regexp.Compile(`\d+`) // Compiled in each iteration + if r.MatchString(d) { + // Process matched data + } + } +} +``` + +#### Correct: + +```go +var dataRegex = regexp.MustCompile(`\d+`) // Compiled once + +func processData(data []string) { + for _, d := range data { + if dataRegex.MatchString(d) { + // Process matched data + } + } +} +``` diff --git a/docs/rfc/template.md b/docs/rfc/template.md new file mode 100644 index 0000000..9e1a0f5 --- /dev/null +++ b/docs/rfc/template.md @@ -0,0 +1,54 @@ +# RFC: [Rule Name] + +## Summary + +[Brief description of the rule] + +## Motivation + +[Why this rule is needed and what problem it solves] + +## Proposed Implementation + +[Detailed explanation of how the rule will work] + +### Rule Details + +- **Rule ID**: [unique identifier] +- **Severity**: [error/warning/info] +- **Category**: [e.g., style, bug prevention, performance, etc.] +- **Auto-fixable**: [whether the rule can be automatically fixed] + +### Code Examples + +#### Incorrect: + +[Example of code that violates the rule] + +#### Correct: + +[Example of code that does not violate the rule] + +### Exceptions + +[Any exceptions or special cases where the rule should not be applied] + +## Alternatives + +[Other approaches or implementations considered] + +## Implementation Impact + +[Potential effects (positive/negative) of introducing this rule] + +## Implementation Plan + +[Step-by-step plan for implementing the rule] + +## Open Questions + +[Unresolved issues or questions that need addressing] + +## References + +[Links to related documents, papers, or existing implementations] diff --git a/docs/rfc/unnecesary-slice-length.md b/docs/rfc/unnecesary-slice-length.md new file mode 100644 index 0000000..b2c0561 --- /dev/null +++ b/docs/rfc/unnecesary-slice-length.md @@ -0,0 +1,53 @@ +# RFC: Unnecessary Slice Length + +## Summary + +This lint rule detects unnecessary use of the `len()` function in slice expressions, where the slice operation can be simplified without affecting the behavior of the code. + +## Motivation + +Go provides a concise syntax for slicing operations. However, developers sometimes use unnecessary `len()` calls when slicing to the end of a slice. This can make the code less readable and potentially less performant. By identifying these instances, we can help developers write more idiomatic and efficient Go code. + +## Proposed Implementation + +The implementation consists of a function `DetectUnnecessarySliceLength` that inspects the AST of a Go file to find slice expressions that can be simplified. The core logic includes: + +1. Inspecting the AST for slice expressions. +2. Checking if the high bound of the slice is a `len()` call. +3. Verifying if the `len()` call argument matches the slice being operated on. +4. Generating appropriate suggestions and messages for different slice patterns. + +### Rule Details + +- **Rule ID**: simplify-slice-range +- **Severity**: warning +- **Category**: style +- **Auto-fixable**: Yes +- **Description**: Detects unnecessary use of `len()` in slice expressions that can be simplified. + +### Code Examples + +#### Incorrect: + +```go +slice := []int{1, 2, 3, 4, 5} +subSlice1 := slice[:len(slice)] // Unnecessary len() +subSlice2 := slice[2:len(slice)] // Unnecessary len() +subSlice3 := slice[start:len(slice)] // Unnecessary len() +``` + +#### Correct: + +```go +slice := []int{1, 2, 3, 4, 5} +subSlice1 := slice[:] // Simplified +subSlice2 := slice[2:] // Simplified +subSlice3 := slice[start:] // Simplified +``` + +## Open Questions + +1. Should we consider more complex slice expressions, such as those involving arithmetic operations? +2. How should we handle cases where the slice and the `len()` argument are different variables but might refer to the same underlying array? +3. Should we provide a configuration option to ignore certain patterns or in specific contexts? +4. How do we ensure that this rule doesn't produce false positives in cases where the explicit use of `len()` might be preferred for readability or documentation purposes? diff --git a/docs/rfc/unnecessary-break.md b/docs/rfc/unnecessary-break.md new file mode 100644 index 0000000..46aa379 --- /dev/null +++ b/docs/rfc/unnecessary-break.md @@ -0,0 +1,78 @@ +# RFC: Unnecessary Break + +## Summary + +This lint rule detects useless `break` statements at the end of case clauses in `switch` or `select` statements in gno code. + +## Motivation + +In go, `switch` and `select` statements do not fall through to the next case by default, unlike in some other languages. This means that a `break` statement at the end of a case clause is unnecessary and can be safely removed, if gno also follows this behavior. + +Detecting and removing these useless `break` statements can improve code readability and prevent confusion, especially for developers coming from languages where fall-through is the default behavior. + +## Proposed Implementation + +The implementation consists of a function `DetectUselessBreak` that inspects the AST of a Go file to find useless `break` statements. The core logic includes: + +1. Traversing the AST to find `switch` and `select` statements. +2. For each case clause in these statements, checking if the last statement is an unnecessary `break`. + +### Rule Details + +- **Rule ID**: useless-break +- **Severity**: warning +- **Category**: style +- **Auto-fixable**: Yes +- **Description**: Detects useless break statements at the end of case clauses in switch or select statements. + +### Detailed Explanation of Useless Break Detection + +The rule considers a `break` statement useless if all of the following conditions are met: + +1. It is the last statement in a case clause of a `switch` or `select` statement. +2. It is a simple `break` without a label (`breakStmt.Label == nil`). + +The implementation uses `ast.Inspect` to traverse the AST and looks specifically for `*ast.SwitchStmt` and `*ast.SelectStmt` nodes. For each of these nodes, it iterates through the case clauses (`*ast.CaseClause` for switch and `*ast.CommClause` for select) and checks the last statement in each clause. + +### Code Examples + +#### Incorrect: + +```go +switch x { +case 1: + println("One") + break // Useless break +case 2: + println("Two") + break // Useless break +default: + println("Other") + break // Useless break +} +``` + +#### Correct: + +```go +switch x { +case 1: + println("One") +case 2: + println("Two") +default: + println("Other") +} +``` + +## Implementation Impact + +- Positive: Improved code readability and reduced confusion. +- Negative: Minimal. Removing these statements does not change the behavior of the code. + +## Open Questions + +1. Should we extend the rule to detect useless `break` statements in `for` loops as well? +2. How should we handle cases where the `break` statement is preceded by a comment? Should we preserve the comment? +3. Should we provide a configuration option to ignore certain patterns or in specific contexts? +4. How do we ensure that this rule doesn't interfere with more complex control flow scenarios where a `break` might be used for clarity, even if it's technically unnecessary? diff --git a/docs/rfc/unnecessary-type-conversion.md b/docs/rfc/unnecessary-type-conversion.md new file mode 100644 index 0000000..1ab39ef --- /dev/null +++ b/docs/rfc/unnecessary-type-conversion.md @@ -0,0 +1,82 @@ +# RFC: Unnecessary Type Conversion + +## Summary + +This lint rule detects unnecessary type conversions in gno code, where the type of the expression being converted is identical to the target type of the conversion. + +## Motivation + +Unnecessary type conversions can untidy code and potentially impact performance due to the unnecessary cost of the conversion. By identifying these problems, we can help developers write cleaner code. + +## Proposed Implementation + +The implementation consists of a function `DetectUnnecessaryConversions` that uses Go's `types` package to perform type checking and analysis. The core logic includes: + +1. Building a type information structure for the entire file. +2. Collecting variable declarations. +3. Inspecting the AST for type conversion expressions. +4. Checking if the conversion is unnecessary based on type identity. + +### Rule Details + +- **Rule ID**: unnecessary-type-conversion +- **Severity**: warning +- **Category**: style/performance +- **Auto-fixable**: Yes +- **Description**: Detects type conversions where the source and target types are identical. + +### Detailed Explanation of Unnecessary Type Conversion Detection + +The rule considers a type conversion unnecessary if all of the following conditions are met: + +1. The expression is a function call-like syntax (`ast.CallExpr`) with exactly one argument. +2. The function being called is actually a type. +3. The type of the argument is identical to the target type of the conversion. +4. The argument is not an untyped constant. + +The last condition is crucial because untyped constants can be implicitly converted, and explicit type conversions for these are sometimes necessary for clarity or to force a specific type. + +The `isUntypedValue` function performs a deep inspection of expressions to determine if they result in an untyped value. It checks various cases including: + +- Certain binary operations (e.g., shifts, comparisons) +- Unary operations +- Basic literals +- Identifiers referring to untyped constants or the nil value +- Calls to certain builtin functions + +This comprehensive check ensures that the rule doesn't falsely flag necessary type conversions of untyped constants. + +### Code Examples + +#### Incorrect: + +```go +var x int = 5 +y := int(x) // Unnecessary conversion + +var s string = "hello" +t := string(s) // Unnecessary conversion +``` + +#### Correct: + +```go +var x int = 5 +y := x // No conversion needed + +var s string = "hello" +t := s // No conversion needed + +const untyped = 5 +typed := int(untyped) // Necessary conversion from untyped to typed +``` + +## Implementation Impact + +- Positive: Cleaner code. +- Negative: The lint process may be slower due to the need for full type checking. + +## Open Questions + +1. Should we extend the rule to check for unnecessary conversions in more complex expressions, such as function call arguments or return values? +2. How do we ensure good performance of this lint rule, given that it requires full type checking of the file?