Fix kinds and stmt #1613

birunts · 2023-08-17T18:12:26Z

What type of PR is this?
Bug fix

What package or component does this PR mostly affect?
internal/language
rule
tests

What does this PR do? Why is it needed?
Bazel allows to pack rules or macros within a struct, e.g. convenience interfaces for select()
(see https://github.com/bazelbuild/bazel-skylib/blob/main/lib/selects.bzl)

Generated rule of selects.config_setting_group by rule.NewRule() is different with the rule parsed from the input file i.e. ruleFromExpr() in a way of different kind and identifier when fixing loads (FixLoads()).

Newly generated rule:

r := rule.NewRule("selects.config_setting_group", "config_group_name")

together with loads:

var loads = []rule.LoadInfo{
	{
		Name: "@bazel_skylib//lib:selects.bzl",
		Symbols: []string{
			"selects",
		},
	},
}

should produce in the BUILD.bazel output::

load("@bazel_skylib//lib:selects.bzl", "selects")

selects.config_setting_group(
    name = "config_group_name",
    ...
)

Which issues(s) does this PR fix?

Fixes #1612

Other notes for review

Applies patch of Gazelle PR #1613 which is not merged yet. This change unifies kinds for two ways of creating rules and correctly sets the identifier while fixing loads (`FixLoads()`). bazel-contrib/bazel-gazelle#1613 Signed-off-by: Sebastian Birunt <sebastian.birunt@arm.com> Change-Id: Iec5ba6dfdec5d90722684b299844184d974a29e7

rule/rule.go

fmeum

Looks good now, with one more comment.

Please mention me after making your changes as I don't get notifications for pushes on PRs.

rule/rule.go

Created rule with `NewRule()` is different to the rule created with `ruleFromExpr()` (when parsing input data). In the first case (`NewRule()`) kind is set to `build.Ident{Name: string}` while it's parsed expression in the second case (`ruleFromExpr()`). For rules packed in a struct like `selects.config_setting_group` there is no way to create/generate the same rule as the one parsed from the file as both will have different kinds. This change unifies kinds for two ways of creating rules and correctly sets the identifier while fixing loads (`FixLoads()`).

Change adds test to verify fixing load statements while generating packed rules in struct e.g. `selects.config_setting_group`.

fmeum

Looks great, thanks!

* fix: correct rule's kind & stmt Created rule with `NewRule()` is different to the rule created with `ruleFromExpr()` (when parsing input data). In the first case (`NewRule()`) kind is set to `build.Ident{Name: string}` while it's parsed expression in the second case (`ruleFromExpr()`). For rules packed in a struct like `selects.config_setting_group` there is no way to create/generate the same rule as the one parsed from the file as both will have different kinds. This change unifies kinds for two ways of creating rules and correctly sets the identifier while fixing loads (`FixLoads()`). * chore: fix load tests Change adds test to verify fixing load statements while generating packed rules in struct e.g. `selects.config_setting_group`. * chore: update MODULE.bazel.lock

birunts force-pushed the fix_kinds_and_stmt branch 2 times, most recently from 02e2418 to b6848e9 Compare August 17, 2023 21:01

birunts mentioned this pull request Aug 22, 2023

Remove load() statement workaround ARM-software/bob-build#655

Merged

fmeum reviewed Aug 28, 2023

View reviewed changes

rule/rule.go Outdated Show resolved Hide resolved

birunts force-pushed the fix_kinds_and_stmt branch 3 times, most recently from efe4aa6 to dce3790 Compare September 8, 2023 11:48

fmeum approved these changes Sep 13, 2023

View reviewed changes

rule/rule.go Outdated Show resolved Hide resolved

birunts force-pushed the fix_kinds_and_stmt branch from dce3790 to fe5bf9b Compare September 21, 2023 10:22

fmeum reviewed Sep 21, 2023

View reviewed changes

rule/rule.go Outdated Show resolved Hide resolved

birunts added 3 commits September 22, 2023 10:10

chore: fix load tests

535d902

Change adds test to verify fixing load statements while generating packed rules in struct e.g. `selects.config_setting_group`.

chore: update MODULE.bazel.lock

5e99d90

birunts force-pushed the fix_kinds_and_stmt branch from fe5bf9b to 5e99d90 Compare September 22, 2023 08:11

fmeum approved these changes Sep 22, 2023

View reviewed changes

fmeum enabled auto-merge (squash) September 22, 2023 21:03

fmeum merged commit 2090e57 into bazel-contrib:master Sep 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix kinds and stmt #1613

Fix kinds and stmt #1613

birunts commented Aug 17, 2023

fmeum left a comment •

edited

Loading

fmeum left a comment

Fix kinds and stmt #1613

Fix kinds and stmt #1613

Conversation

birunts commented Aug 17, 2023

fmeum left a comment • edited Loading

Choose a reason for hiding this comment

fmeum left a comment

Choose a reason for hiding this comment

fmeum left a comment •

edited

Loading