Skip to content

Conversation

kakkoyun
Copy link
Contributor

@kakkoyun kakkoyun commented Aug 6, 2025

  • Implement conflict resolution when both type assertion and error comparison linting rules apply to the same line
  • Add support for else-if statement transformations using errors.As and errors.Is
  • Generate combined diagnostics with unified suggested fixes

Signed-off-by: Kemal Akkoyun [email protected]

fixes polyfloyd#100)

- Implement conflict resolution when both type assertion and error comparison linting rules apply to the same line
- Add support for else-if statement transformations using errors.As and errors.Is
- Generate combined diagnostics with unified suggested fixes

Signed-off-by: Kemal Akkoyun <[email protected]>
@kakkoyun kakkoyun mentioned this pull request Aug 6, 2025
@rockdaboot
Copy link

Testing this PR on the same code as in the error description, the comment lines were dropped.

-               } else if e, ok := err.(*os.PathError); ok && e.Err == syscall.ESRCH {
-                       // If the process exits while reading its /proc/$PID/maps, the kernel will
-                       // return ESRCH. Handle it as if the process did not exist.
-                       pm.mappingStats.errProcESRCH.Add(1)
+               } else {
+                       e := &os.PathError{}
+                       if errors.As(err, &e) && errors.Is(e.Err, syscall.ESRCH) {
+                               pm.mappingStats.errProcESRCH.Add(1)
+                       }
                }

Also (maybe a different issue), with the following change, the errors import is missing

-       if n == 0 || (err != nil && err != io.EOF) {
+       if n == 0 || (err != nil && !errors.Is(err, io.EOF)) {

Let me know if this is unrelated and I'll open another issue.

@kakkoyun
Copy link
Contributor Author

Testing this PR on the same code as in the error description, the comment lines were dropped.

-               } else if e, ok := err.(*os.PathError); ok && e.Err == syscall.ESRCH {
-                       // If the process exits while reading its /proc/$PID/maps, the kernel will
-                       // return ESRCH. Handle it as if the process did not exist.
-                       pm.mappingStats.errProcESRCH.Add(1)
+               } else {
+                       e := &os.PathError{}
+                       if errors.As(err, &e) && errors.Is(e.Err, syscall.ESRCH) {
+                               pm.mappingStats.errProcESRCH.Add(1)
+                       }
                }

Also (maybe a different issue), with the following change, the errors import is missing

-       if n == 0 || (err != nil && err != io.EOF) {
+       if n == 0 || (err != nil && !errors.Is(err, io.EOF)) {

Let me know if this is unrelated and I'll open another issue.

Good catch. I have to make sure we preserve the comments (hard with stdlib).

I will think ways to simplify the implementation. It is hard to rewrite control flow statements.

@kakkoyun kakkoyun marked this pull request as draft August 11, 2025 11:53
Signed-off-by: Kemal Akkoyun <[email protected]>
@polyfloyd
Copy link
Owner

Hey @kakkoyun, is this still considered a draft? Or is this getting to a state where it can be reviewed?

@kakkoyun
Copy link
Contributor Author

kakkoyun commented Sep 9, 2025

Hey @kakkoyun, is this still considered a draft? Or is this getting to a state where it can be reviewed?

It is technically in a reviewable state. I can mark it ready. However, I want to do some clean up and refactoring.

What would you prefer? Merge this and iterate or have the refactoring in this PR?

@polyfloyd
Copy link
Owner

Yes, please include the refactoring in this PR 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants