Add gounqvet linter #6060

MirrexOne · 2025-09-06T01:33:30Z

This PR adds gounqvet as a new linter to report SELECT * usage in SQL queries and SQL builders, encouraging explicit column selection for better performance, maintainability, and API stability.

Repository: https://github.com/MirrexOne/gounqvet

CLAassistant · 2025-09-06T01:33:37Z

All committers have signed the CLA.

boring-cyborg · 2025-09-06T01:33:38Z

Hey, thank you for opening your first Pull Request !

ldez · 2025-09-06T01:34:42Z

In order for a pull request adding a linter to be reviewed, the linter and the PR must follow some requirements.

The CLA must be signed

Pull Request Description

It must have a link to the linter repository.
It must provide a short description of the linter.

Linter

The Linter Tests Inside Golangci-lint

They must have at least one std lib import.
They must have integration tests without configuration (default).
They must have integration tests with configuration (if the linter has a configuration).

`.golangci.next.reference.yml`

The file .golangci.next.reference.yml must be updated.
The file .golangci.reference.yml must NOT be edited.
The linter must be added to the lists of available linters (alphabetical case-insensitive order).
- enable and disable options
If the linter has a configuration, the exhaustive configuration of the linter must be added (alphabetical case-insensitive order)
- The values must be different from the default ones.
- The default values must be defined in a comment.
- The option must have a short description.

Others Requirements

The files (tests and linter) inside golangci-lint must have the same name as the linter.
The .golangci.yml of golangci-lint itself must not be edited and the linter must not be added to this file.
The linters must be sorted in the alphabetical order (case-insensitive) in the lintersdb/builder_linter.go and .golangci.next.reference.yml.
The load mode (WithLoadMode(...)):
- if the linter uses goanalysis.LoadModeSyntax -> no WithLoadForGoAnalysis() in lintersdb/builder_linter.go
- if the linter uses goanalysis.LoadModeTypesInfo, it requires WithLoadForGoAnalysis() in lintersdb/builder_linter.go
The version in WithSince(...) must be the next minor version (v2.X.0) of golangci-lint.
WithURL() must contain the URL of the repository.

Recommendations

The file jsonschema/golangci.next.jsonschema.json should be updated.
The file jsonschema/golangci.jsonschema.json must NOT be edited.
The linter repository should have a readme and linting.
The linter should be published as a binary (useful for diagnosing bug origins).
The linter repository should have a .gitignore (IDE files, binaries, OS files, etc. should not be committed)
A tag should never be recreated.
use main as the default branch name.

The golangci-lint team will edit this comment to check the boxes before and during the review.

The code review will start after the completion of those checkboxes (except for the specific items that the team will help to verify).

The reviews should be addressed as commits (no squash).

If the author of the PR is a member of the golangci-lint team, he should not edit this message.

This checklist does not imply that we will accept the linter.

ldez · 2025-09-06T01:36:25Z

Hello,

Inside golangci-lint we differentiate 2 types of analyzers:

the linters (find problems with nearly 0 false positives)
the detectors (detect potential problems, and so a majority of false positives)

The detectors are not allowed because of the number of false positives.

My initial impression is that your analyzer is a detector, not a linter.

If I'm wrong, please add a comment to explain why your analyzer is not a detector.

ldez · 2025-09-06T01:51:32Z

Please short message, and no answer from AI.

I'm a human, I'm talking to humans.

MirrexOne · 2025-09-06T01:58:02Z

This is not AI - that's part of my speech on work presentation, but sorry for long read.

This linter exists because my team had a critical production incident caused by SELECT * - performance degradation and cascading service failures when new columns were added. This could have
been prevented with this linter.

Like gosec (detects security issues) or sqlclosecheck (detects resource leaks), gounqvet finds objective problems with measurable impact, not style preferences.

This is the first iteration with more features planned. I'd be extremely grateful if you could consider this - we want to adopt it properly through golangci-lint rather than standalone binaries.

New features will be added in the near future, as there is a need for expansion. This linter will grow

ldez · 2025-09-06T02:02:56Z

gosec reports security issues, sqlclosecheck reports resource leaks.

"detect" is not the right word:

Detection is related to informative elements
Reporting is related to important elements.

Note: I don't add a linter because it's "critical" for someone, but if it's useful for the community.

ldez · 2025-09-06T02:08:32Z

Every option related to ignore should be removed from the golangci-lint configuration for this linter:
- ignored-packages
- ignored-file-patterns
- ignored-directories
The min Go version inside your go.mod must be go1.24.0.
All the files inside golangci-lint should have an empty line at the end.
The file pkg/golinters/gounqvet/gounqvet_test.go should be removed.
The file pkg/golinters/gounqvet/testdata/gounqvet.go doesn't work because of missing directives.
The alphabetical order is not followed (u is after s).
This linter doesn't required type information.

MirrexOne · 2025-09-06T02:11:09Z

gosec reports security issues, sqlclosecheck reports resource leaks.

"detect" is not the right word:

Detection is related to informative elements

Reporting is related to important elements.

Note: I don't add a linter because it's "critical" for someone, but if it's useful for the community.

Yes, I agree with your opinion, but my company has a large Go community that likes this tool and many have adopted it in their projects, both work and open source, so it applies to more than just 1-2 people.

I'm not trying to argue, but I would like to convey that this tool is meaningful and useful for the community, and over time and with the addition of new features, it will become quite flexible in terms of configuration.

ldez · 2025-09-06T02:40:06Z

No AI... but your commits are co-authored by Claude...

MirrexOne · 2025-09-06T02:41:38Z

No AI... but your commits are co-authored by Claude...

AI as helper for coding, not for responses

ldez · 2025-09-06T02:43:36Z

It doesn't feel very helpful with code: https://github.com/MirrexOne/gounqvet/actions/runs/17508774320/job/49736927221

MirrexOne · 2025-09-06T02:48:51Z

It doesn't feel very helpful with code: https://github.com/MirrexOne/gounqvet/actions/runs/17508774320/job/49736927221

Yes, sure. That was redundant.

Already removed it

MirrexOne · 2025-09-06T08:24:55Z

Please let me know if you have any further comments on any of the points ?

This PR adds gounqvet as a new linter to detect SELECT * usage in SQL queries and SQL builders, encouraging explicit column selection for better performance, maintainability, and API stability. ## Features - **SQL String Detection**: Finds SELECT * in string literals - **SQL Builder Support**: Works with popular builders like Squirrel, GORM - **Smart Pattern Recognition**: Avoids false positives for COUNT(*), system queries - **Highly Configurable**: Extensive configuration options - **Standard Integration**: Supports //nolint:gounqvet directives - **Zero Dependencies**: Built on go/analysis framework ## Configuration Options ```yaml linters-settings: gounqvet: check-sql-builders: true # Enable SQL builder checking ignored-functions: [] # Functions to skip ignored-packages: [] # Packages to ignore allowed-patterns: [] # Regex for acceptable SELECT * ignored-file-patterns: [] # File patterns to ignore ignored-directories: [] # Directories to ignore ``` ## Why Avoid SELECT *? - **Performance**: Reduces unnecessary network bandwidth and memory usage - **Maintainability**: Prevents breakage when schema changes - **Security**: Avoids exposing sensitive columns unintentionally - **API Stability**: Prevents issues when new columns are added ## Examples **Problematic code (detected):** ```go query := "SELECT * FROM users" rows := squirrel.Select("*").From("products") ``` **Good code (not flagged):** ```go query := "SELECT id, name FROM users" rows := squirrel.Select("id", "name").From("products") count := "SELECT COUNT(*) FROM users" // Aggregate functions OK ``` Available since golangci-lint v1.50.0 for maximum compatibility. Repository: https://github.com/MirrexOne/gounqvet

- Update WithSince to v2.5.0 (next minor version as required) - Add gounqvet to .golangci.next.reference.yml in alphabetical order - Include comprehensive configuration with non-default values - Place linter in correct alphabetical position in builder_linter.go - Add detailed configuration comments and descriptions All compliance requirements now met according to checklist.

- Add strconv import and usage to satisfy checklist requirement - Tests must have at least one std lib import

- Remove all ignore-related options from configuration - Remove gounqvet_test.go as requested - Fix testdata directives format (move 'want' to end of line) - Remove ignore fields from GounqvetSettings struct - Change WithLoadForGoAnalysis to LoadModeSyntax (no type info needed) - Add empty lines at end of all files - Simplify configuration to only check-sql-builders and allowed-patterns All technical feedback addressed.

ldez · 2025-09-11T20:18:31Z

You don't need to rebase unless there are conflicts.

Each time you rebase, I must launch the CI manually.

MirrexOne · 2025-09-11T20:21:50Z

You don't need to rebase unless there are conflicts.
Each time you rebase, I must launch the CI manually.

Thank you for pointing that out. I will no longer rebase unnecessarily.

ldez · 2025-09-12T14:48:24Z

I have some doubts about the linter name: gounqvet.
I don't see the link between SQL select and this name.

MirrexOne · 2025-09-12T19:47:09Z

I have some doubts about the linter name: gounqvet. I don't see the link between SQL select and this name.

I decided not to get too hung up on “select *” specifically, since there is already a need to check more than just one construct.

MirrexOne · 2025-09-12T19:53:46Z

As practice and feedback from those who already use linter in my company have shown, this is a fairly idiomatic name for Go.

ldez added linter: new Support new linter feedback required Requires additional feedback labels Sep 6, 2025

ldez self-requested a review September 6, 2025 01:37

This comment was marked as off-topic.

Sign in to view

This comment was marked as outdated.

Sign in to view

MirrexOne force-pushed the add-gounqvet-linter branch from 09c96df to bd42b77 Compare September 6, 2025 02:39

MirrexOne force-pushed the add-gounqvet-linter branch from bd42b77 to 0dc997d Compare September 6, 2025 02:43

ldez changed the title ~~Add gounqvet linter for detecting SELECT * usage~~ Add gounqvet linter Sep 7, 2025

ldez force-pushed the add-gounqvet-linter branch from 27136e0 to 3817c96 Compare September 8, 2025 00:53

ldez removed the feedback required Requires additional feedback label Sep 8, 2025

MirrexOne force-pushed the add-gounqvet-linter branch from 8cde68e to 2e1ba6d Compare September 8, 2025 21:33

ldez added this to the v2-unreleased milestone Sep 11, 2025

MirrexOne added 3 commits September 11, 2025 23:17

Add std lib import requirement to testdata

691a0f5

- Add strconv import and usage to satisfy checklist requirement - Tests must have at least one std lib import

MirrexOne and others added 6 commits September 11, 2025 23:17

Update gounqvet to v1.2.0 - remove nolint support

d7ef48f

Add missing golangcitest directive and integration test

ea71a60

Add gounqvet_integration_test.go and remove old test file

78dcd8b

review

9cf3ad7

review

50818d5

MirrexOne force-pushed the add-gounqvet-linter branch from 2e1ba6d to 50818d5 Compare September 11, 2025 20:17

ldez added the feedback required Requires additional feedback label Sep 12, 2025

Uh oh!

Add gounqvet linter #6060

Are you sure you want to change the base?

Add gounqvet linter #6060

Uh oh!

Conversation

MirrexOne commented Sep 6, 2025 • edited by ldez Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boring-cyborg bot commented Sep 6, 2025

Uh oh!

ldez commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Description

Linter

The Linter Tests Inside Golangci-lint

.golangci.next.reference.yml

Others Requirements

Recommendations

Uh oh!

ldez commented Sep 6, 2025

Uh oh!

This comment was marked as off-topic.

ldez commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MirrexOne commented Sep 6, 2025

Uh oh!

ldez commented Sep 6, 2025

Uh oh!

ldez commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MirrexOne commented Sep 6, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

ldez commented Sep 6, 2025

Uh oh!

MirrexOne commented Sep 6, 2025

Uh oh!

ldez commented Sep 6, 2025

Uh oh!

MirrexOne commented Sep 6, 2025

Uh oh!

MirrexOne commented Sep 6, 2025

Uh oh!

ldez commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MirrexOne commented Sep 11, 2025

Uh oh!

ldez commented Sep 12, 2025

Uh oh!

MirrexOne commented Sep 12, 2025

Uh oh!

MirrexOne commented Sep 12, 2025

Uh oh!

Uh oh!

MirrexOne commented Sep 6, 2025 •

edited by ldez

Loading

CLAassistant commented Sep 6, 2025 •

edited

Loading

ldez commented Sep 6, 2025 •

edited

Loading

`.golangci.next.reference.yml`

ldez commented Sep 6, 2025 •

edited

Loading

ldez commented Sep 6, 2025 •

edited

Loading

ldez commented Sep 11, 2025 •

edited

Loading