Skip to content

convert: lazily create ConstGroup in createEnumItems to avoid empty const nodes#644

Merged
MeteorsLiu merged 3 commits into
mainfrom
xgopilot/issue-643-1770885378
Feb 12, 2026
Merged

convert: lazily create ConstGroup in createEnumItems to avoid empty const nodes#644
MeteorsLiu merged 3 commits into
mainfrom
xgopilot/issue-643-1770885378

Conversation

@fennoai
Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot commented Feb 12, 2026

Requested by @luoliwoshang

Closes #643

Summary

  • Lazily initialize ConstGroup in createEnumItems so that p.NewConstGroup() is only called when an enum item is actually emitted
  • If all enum items are skipped (e.g., due to name conflicts or filtering), no empty const group is created
  • This avoids unnecessary empty const () nodes in generated output

Changes

  • cl/internal/convert/package.go: Replaced eager defs := p.NewConstGroup() with a lazy ensureDefs() closure that creates the group on first use

…onst nodes

Use lazy initialization for ConstGroup in createEnumItems: only create
p.NewConstGroup() when an enum item is actually emitted. This avoids
unnecessary empty const declarations/nodes when all enum items are
skipped due to name conflicts or filtering.

Closes #643

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <51194195+luoliwoshang@users.noreply.github.com>
@luoliwoshang
Copy link
Copy Markdown
Contributor

@xgopilot make a unit_test to mock this behavior will not create a empty const when have not valid enum item, you can make it at /cl/internal/convert/package_bulitin_test.go!

@fennoai
Copy link
Copy Markdown
Contributor Author

fennoai Bot commented Feb 12, 2026

Added unit test TestNoEmptyConstGroupWhenAllEnumItemsSkipped in cl/internal/convert/package_bulitin_test.go.

What the test does

  1. Creates a package and registers enum items (Red, Green) via a first NewEnumTypeDecl("Color", ...) call
  2. Calls NewEnumTypeDecl("Color2", ...) with the same items — all items are already registered, so all are skipped in createEnumItems
  3. Captures the generated output via WriteTo and asserts there is exactly 1 const ( block (from the first enum), with no empty const () from the second enum

Verification

  • go test -run TestNoEmptyConstGroupWhenAllEnumItemsSkippedPASS
  • go fmt and go vet — clean
  • Pre-existing test failures (TestFromTestdata, TestDepWithVersion, TestSysToPkg, TestDepPkg) are unrelated and require install.sh

Commit: 3dfc858

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.77%. Comparing base (26d0726) to head (845eb48).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #644      +/-   ##
==========================================
+ Coverage   84.74%   84.77%   +0.02%     
==========================================
  Files          27       27              
  Lines        2314     2318       +4     
==========================================
+ Hits         1961     1965       +4     
  Misses        310      310              
  Partials       43       43              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ipped

Adds TestNoEmptyConstGroupWhenAllEnumItemsSkipped to verify that
no empty const() block is generated when all enum items are already
registered and thus skipped in createEnumItems.

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <51194195+luoliwoshang@users.noreply.github.com>
constCount := strings.Count(output, "const (")
if constCount != 1 {
t.Errorf("expected exactly 1 const block, got %d.\nOutput:\n%s", constCount, output)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xgopilot also need check the const Decl not have the another at pkg corresponding ast.File , cover this behavior!

Copy link
Copy Markdown
Contributor Author

@fennoai fennoai Bot Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enhanced the test to also verify at the Go AST level that no empty const GenDecl exists in the package's ast.File.

What was added

After the existing strings.Count check on the text output, the test now also:

  1. Gets the Go AST file via pkg.p.ASTFile("temp.go")
  2. Iterates over all declarations and checks that no *goast.GenDecl with Tok == token.CONST has empty Specs
  3. Asserts there is exactly 1 const GenDecl in the AST (from the first enum only)

Verification

  • go test -run TestNoEmptyConstGroupWhenAllEnumItemsSkippedPASS
  • go fmt and go vet — clean

Commit: 845eb48

Enhance TestNoEmptyConstGroupWhenAllEnumItemsSkipped to also verify
at the Go AST level that no const GenDecl with empty Specs exists
in the package's corresponding ast.File, covering the behavior that
lazy ConstGroup initialization avoids empty const nodes in the AST.

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <51194195+luoliwoshang@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@luoliwoshang luoliwoshang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MeteorsLiu MeteorsLiu merged commit d59271a into main Feb 12, 2026
12 checks passed
@fennoai fennoai Bot deleted the xgopilot/issue-643-1770885378 branch February 12, 2026 10:23
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.

convert: lazily create ConstGroup in createEnumItems to avoid empty const nodes

3 participants