Skip to content

refactor: remove unnecessary unsafe slice modification#201

Merged
jbedard merged 1 commit intomainfrom
shared-json
Feb 14, 2026
Merged

refactor: remove unnecessary unsafe slice modification#201
jbedard merged 1 commit intomainfrom
shared-json

Conversation

@jbedard
Copy link
Member

@jbedard jbedard commented Feb 12, 2026

Changes are visible to end-users: no

Test plan

  • Covered by existing test cases

Copilot AI review requested due to automatic review settings February 12, 2026 21:37
@aspect-workflows
Copy link

aspect-workflows bot commented Feb 12, 2026

Test

All tests were cache hits

4 tests (100.0%) were fully cached saving 321ms.


Test

language/js

118 test targets passed

Targets
//tests:bad_build_test [k8-fastbuild]45ms
//tests:bad_lockfile_test [k8-fastbuild]69ms
//tests:bad_package_json-invalid_test [k8-fastbuild]151ms
//tests:bad_package_json_test [k8-fastbuild]178ms
//tests:bad_ts_syntax_test [k8-fastbuild]165ms
//tests:declare_module_test [k8-fastbuild]298ms
//tests:declare_module_types_test [k8-fastbuild]193ms
//tests:dynamic_import_test [k8-fastbuild]105ms
//tests:gazelle_disable_conflict_test [k8-fastbuild]84ms
//tests:gazelle_exclude_directive_test [k8-fastbuild]245ms
//tests:gazelle_generate_build_test [k8-fastbuild]105ms
//tests:gazelle_generation_mode_legacy_test [k8-fastbuild]81ms
//tests:gazelle_generation_mode_test [k8-fastbuild]217ms
//tests:gazelle_keep_test [k8-fastbuild]175ms
//tests:gazelle_map_kind_directive_test [k8-fastbuild]194ms
//tests:groups_configs_test [k8-fastbuild]126ms
//tests:groups_deps_test [k8-fastbuild]174ms
//tests:groups_simple_files_test [k8-fastbuild]151ms
//tests:ignore_config_files_test [k8-fastbuild]141ms
//tests:ignore_import_directive_test [k8-fastbuild]102ms
//tests:incremental_lazy_lockdir_test [k8-fastbuild]117ms
//tests:incremental_lazy_test [k8-fastbuild]168ms
//tests:isolated_typecheck_test [k8-fastbuild]101ms
//tests:node_native_test [k8-fastbuild]217ms
//tests:npm_link_all_packages_test [k8-fastbuild]63ms
//tests:npm_package_deps_lib_test [k8-fastbuild]187ms
//tests:npm_package_deps_test [k8-fastbuild]169ms
//tests:npm_package_deps_tsconfig_test [k8-fastbuild]166ms
//tests:npm_package_lib_target_name_test [k8-fastbuild]219ms
//tests:npm_package_target_enabled_test [k8-fastbuild]133ms
//tests:npm_package_target_name_test [k8-fastbuild]155ms
//tests:npm_package_target_referenced_test [k8-fastbuild]170ms
//tests:npm_simple_deps_cjs_test [k8-fastbuild]134ms
//tests:npm_simple_deps_test [k8-fastbuild]290ms
//tests:npm_types_package_test [k8-fastbuild]207ms
//tests:parse_errors_test [k8-fastbuild]221ms
//tests:pnpm_project_refs_lock5_test [k8-fastbuild]157ms
//tests:pnpm_project_refs_lock6_test [k8-fastbuild]199ms
//tests:pnpm_project_refs_lock9_test [k8-fastbuild]139ms
//tests:pnpm_workspace-incremental_test [k8-fastbuild]221ms
//tests:pnpm_workspace_rerooted_test [k8-fastbuild]212ms
//tests:pnpm_workspace_test [k8-fastbuild]152ms
//tests:resolve_directive_test [k8-fastbuild]178ms
//tests:resolve_order_test [k8-fastbuild]210ms
//tests:rules_conflicting_name_mapped_kind_test [k8-fastbuild]93ms
//tests:rules_conflicting_name_nojs_test [k8-fastbuild]56ms
//tests:rules_conflicting_name_test [k8-fastbuild]81ms
//tests:rules_ordering_test [k8-fastbuild]127ms
//tests:simple_dts_only_dep_test [k8-fastbuild]178ms
//tests:simple_dts_only_test [k8-fastbuild]210ms
//tests:simple_empty_test [k8-fastbuild]81ms
//tests:simple_file_test [k8-fastbuild]179ms
//tests:simple_globs_test [k8-fastbuild]175ms
//tests:simple_import_generated_test [k8-fastbuild]170ms
//tests:simple_imports_cjs_test [k8-fastbuild]162ms
//tests:simple_imports_dynamic_test [k8-fastbuild]117ms
//tests:simple_imports_test [k8-fastbuild]306ms
//tests:simple_json_import_test [k8-fastbuild]166ms
//tests:simple_module_repo_name_test [k8-fastbuild]104ms
//tests:simple_module_test [k8-fastbuild]169ms
//tests:simple_new_file_test [k8-fastbuild]253ms
//tests:simple_rule_naming_directives_test [k8-fastbuild]143ms
//tests:source_no_target_err_test [k8-fastbuild]47ms
//tests:tests_initial_test [k8-fastbuild]186ms
//tests:tests_nolib_test [k8-fastbuild]107ms
//tests:tests_simple_test [k8-fastbuild]158ms
//tests:tests_subproject_test [k8-fastbuild]177ms
//tests:ts_proto_library_deps_test [k8-fastbuild]83ms
//tests:ts_proto_library_error_test [k8-fastbuild]104ms
//tests:ts_proto_library_ignore_test [k8-fastbuild]92ms
//tests:ts_proto_library_imported_test [k8-fastbuild]162ms
//tests:ts_proto_library_test [k8-fastbuild]161ms
//tests:tsconfig_attrs_inherited_test [k8-fastbuild]211ms
//tests:tsconfig_baseurl_test [k8-fastbuild]165ms
//tests:tsconfig_custom_file_name_test [k8-fastbuild]178ms
//tests:tsconfig_declaration_dir_test [k8-fastbuild]165ms
//tests:tsconfig_deps_test [k8-fastbuild]145ms
//tests:tsconfig_disabled_manual_test [k8-fastbuild]171ms
//tests:tsconfig_emit_declaration_only_test [k8-fastbuild]182ms
//tests:tsconfig_incremental_test [k8-fastbuild]181ms
//tests:tsconfig_invalid_test [k8-fastbuild]98ms
//tests:tsconfig_jsx_test [k8-fastbuild]332ms
//tests:tsconfig_lax_json_test [k8-fastbuild]162ms
//tests:tsconfig_noEmit_test [k8-fastbuild]164ms
//tests:tsconfig_nomore_configs_test [k8-fastbuild]182ms
//tests:tsconfig_optout_test [k8-fastbuild]184ms
//tests:tsconfig_outdir_test [k8-fastbuild]148ms
//tests:tsconfig_paths_test [k8-fastbuild]107ms
//tests:tsconfig_pnpm_ref-incremental_test [k8-fastbuild]61ms
//tests:tsconfig_pnpm_ref_rerooted_test [k8-fastbuild]46ms
//tests:tsconfig_pnpm_ref_test [k8-fastbuild]121ms
//tests:tsconfig_rootdir_test [k8-fastbuild]100ms
//tests:tsconfig_rootdirs_test [k8-fastbuild]225ms
//tests:tsconfig_tsbuildinfo_test [k8-fastbuild]211ms
//tests:tsconfig_tslib_test [k8-fastbuild]173ms
//tests:tsx_css_opt-out_test [k8-fastbuild]256ms
//tests:tsx_css_outdir_test [k8-fastbuild]213ms
//tests:validate_import_statements_off_test [k8-fastbuild]106ms
//tests:validate_import_statements_test [k8-fastbuild]192ms
//tests:visibility_test [k8-fastbuild]128ms
+ 18 other targets

Total test execution time was 19s. 6 tests (4.8%) were fully cached saving 2s.


Test

language/kotlin

All tests were cache hits

19 tests (100.0%) were fully cached saving 5s.


Test

language/orion

All tests were cache hits

51 tests (100.0%) were fully cached saving 9s.


Test

runner

14 test targets passed

Targets
//pkg/git/tests:ignore_config_files_test [k8-fastbuild]382ms
//tests:bad_build_test [k8-fastbuild]217ms
//tests:branded-directives_test [k8-fastbuild]345ms
//tests:buf_test [k8-fastbuild]242ms
//tests:cc_test [k8-fastbuild]305ms
//tests:crossresolve-js_test [k8-fastbuild]375ms
//tests:crossresolve-pnpm-names-lazy_test [k8-fastbuild]404ms
//tests:crossresolve-pnpm-names_test [k8-fastbuild]380ms
//tests:crossresolve-pnpm_test [k8-fastbuild]108ms
//tests:golang_test [k8-fastbuild]252ms
//tests:imports-missing_test [k8-fastbuild]224ms
//tests:js_binary-main_test [k8-fastbuild]314ms
//tests:npm_bin_wrapper_test [k8-fastbuild]343ms
//tests:python-simple_test_test [k8-fastbuild]373ms

Total test execution time was 4s. 16 tests (53.3%) were fully cached saving 1s.


Test

runner/e2e/bin

All tests were cache hits

1 test (100.0%) was fully cached saving 93ms.


Buildifier      Gazelle      Gazelle [language/js]      Gazelle [language/kotlin]      Gazelle [language/orion]      Gazelle [runner]      Gazelle [runner/e2e/bin]

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the JS/TS Gazelle rule generation so that package targets (npm_package / js_library when used as the package target kind) always include package.json in their emitted srcs, and refreshes/extends test fixtures to cover the behavior.

Changes:

  • Update addPackageRule generation to always include package.json in the emitted package rule srcs.
  • Update golden BUILD.out fixtures to reflect package.json being included in package targets.
  • Add new/expanded test fixtures (including a pnpm workspace scenario) to exercise JSON/module import behavior.

Reviewed changes

Copilot reviewed 75 out of 81 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
language/js/generate.go Adjusts package rule generation to always emit package.json in package target srcs.
language/js/tests/simple_json_import/shared_json_a.ts Adds shared JSON import test source.
language/js/tests/simple_json_import/shared_json_b.ts Adds shared JSON import test source.
language/js/tests/simple_json_import/BUILD.in Adds gazelle directives for new shared JSON files.
language/js/tests/simple_json_import/BUILD.out Golden update adding new ts_project targets for shared JSON files.
language/js/tests/pnpm_workspace/lib/a/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/pnpm_workspace/lib/b/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/pnpm_workspace/lib/c/BUILD.out Golden update: js_library srcs now include package.json.
language/js/tests/pnpm_workspace_rerooted/lib/a/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/pnpm_workspace_rerooted/lib/b/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/pnpm_workspace_rerooted/lib/c/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/pnpm_workspace_rerooted_subdir/lib/a/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/pnpm_workspace_rerooted_subdir/lib/b/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/pnpm_workspace_rerooted_subdir/lib/c/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/npm_package_target_enabled/lib/a/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/npm_package_target_enabled/lib/b/BUILD.out Golden update: js_library srcs now include package.json.
language/js/tests/npm_package_target_enabled/lib/c/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/npm_package_target_name/lib/b/BUILD.out Golden update: js_library srcs now include package.json.
language/js/tests/npm_package_target_name/lib/c/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/npm_package_target_referenced/lib/b/BUILD.out Golden update: js_library srcs now include package.json.
language/js/tests/npm_package_target_referenced/lib/c/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/npm_package_lib_target_name/lib/b/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/npm_package_lib_target_name/lib/c/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/incremental_lazy/libs/to-update/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/incremental_lazy_lockdir/libs/to-update/BUILD.out Golden update: npm_package srcs now include package.json.
language/js/tests/pnpm_workspace_groups_json_imports/WORKSPACE New test workspace fixture for pnpm workspace groups scenario.
language/js/tests/pnpm_workspace_groups_json_imports/MODULE.bazel New test fixture file for Bzlmod presence.
language/js/tests/pnpm_workspace_groups_json_imports/tsconfig.json New test tsconfig enabling resolveJsonModule.
language/js/tests/pnpm_workspace_groups_json_imports/pnpm-workspace.yaml New pnpm workspace definition for test fixture.
language/js/tests/pnpm_workspace_groups_json_imports/pnpm-lock.yaml New pnpm lockfile for test fixture.
language/js/tests/pnpm_workspace_groups_json_imports/package.json New root package.json for test fixture.
language/js/tests/pnpm_workspace_groups_json_imports/BUILD.in New gazelle directives for test fixture.
language/js/tests/pnpm_workspace_groups_json_imports/BUILD.out New generated root npm_package + ts_config golden output.
language/js/tests/pnpm_workspace_groups_json_imports/lib/a/package.json New fixture package.json for lib a.
language/js/tests/pnpm_workspace_groups_json_imports/lib/a/data.json New fixture JSON data for lib a.
language/js/tests/pnpm_workspace_groups_json_imports/lib/a/config.ts New fixture re-exporting JSON for lib a.
language/js/tests/pnpm_workspace_groups_json_imports/lib/a/index.ts New fixture TS source for lib a.
language/js/tests/pnpm_workspace_groups_json_imports/lib/a/index.spec.ts New fixture test TS source for lib a.
language/js/tests/pnpm_workspace_groups_json_imports/lib/a/BUILD.out New generated BUILD golden for lib a.
language/js/tests/pnpm_workspace_groups_json_imports/lib/a/BUILD.in New/empty BUILD.in fixture for lib a.
language/js/tests/pnpm_workspace_groups_json_imports/lib/b/package.json New fixture package.json for lib b.
language/js/tests/pnpm_workspace_groups_json_imports/lib/b/index.ts New fixture TS source for lib b.
language/js/tests/pnpm_workspace_groups_json_imports/lib/b/data.json New fixture JSON data for lib b.
language/js/tests/pnpm_workspace_groups_json_imports/lib/b/config.ts New fixture TS source requiring JSON/package.json.
language/js/tests/pnpm_workspace_groups_json_imports/lib/b/index.spec.ts New fixture test TS source for lib b.
language/js/tests/pnpm_workspace_groups_json_imports/lib/b/BUILD.out New generated BUILD golden for lib b.
language/js/tests/pnpm_workspace_groups_json_imports/lib/b/BUILD.in New/empty BUILD.in fixture for lib b.
language/js/tests/pnpm_workspace_groups_json_imports/lib/c/package.json New fixture package.json for lib c.
language/js/tests/pnpm_workspace_groups_json_imports/lib/c/index.ts New fixture TS source requiring package.json.
language/js/tests/pnpm_workspace_groups_json_imports/lib/c/data.json New fixture JSON data for lib c.
language/js/tests/pnpm_workspace_groups_json_imports/lib/c/config.ts New fixture TS source requiring package.json.
language/js/tests/pnpm_workspace_groups_json_imports/lib/c/index.spec.ts New fixture test TS source for lib c.
language/js/tests/pnpm_workspace_groups_json_imports/lib/c/BUILD.out New generated BUILD golden for lib c.
language/js/tests/pnpm_workspace_groups_json_imports/lib/c/BUILD.in New gazelle directive for js package kind in lib c.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/BUILD.out New generated ts_config for outdir group.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/BUILD.in New/empty BUILD.in fixture for outdir group.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/tsconfig.json New fixture tsconfig for outDir scenario.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/d/package.json New fixture package.json for lib-outdir d.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/d/data.json New fixture JSON data for lib-outdir d.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/d/config.ts New fixture re-exporting JSON for lib-outdir d.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/d/index.ts New fixture TS source for lib-outdir d.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/d/index.spec.ts New fixture test TS source for lib-outdir d.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/d/BUILD.out New generated BUILD golden for lib-outdir d.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/d/BUILD.in New/empty BUILD.in fixture for lib-outdir d.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/e/package.json New fixture package.json for lib-outdir e.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/e/index.ts New fixture TS source for lib-outdir e.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/e/data.json New fixture JSON data for lib-outdir e.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/e/config.ts New fixture TS source requiring JSON/package.json.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/e/index.spec.ts New fixture test TS source for lib-outdir e.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/e/BUILD.out New generated BUILD golden for lib-outdir e.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/e/BUILD.in New/empty BUILD.in fixture for lib-outdir e.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/f/package.json New fixture package.json for lib-outdir f.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/f/index.ts New fixture TS source requiring package.json.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/f/data.json New fixture JSON data for lib-outdir f.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/f/config.ts New fixture TS source requiring package.json.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/f/index.spec.ts New fixture test TS source for lib-outdir f.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/f/BUILD.out New generated BUILD golden for lib-outdir f.
language/js/tests/pnpm_workspace_groups_json_imports/lib-outdir/f/BUILD.in New gazelle directive for js package kind in lib-outdir f.
Files not reviewed (1)
  • language/js/tests/pnpm_workspace_groups_json_imports/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 159 out of 169 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • language/js/tests/pnpm_workspace_groups_json_imports/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings February 14, 2026 02:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 154 out of 164 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • language/js/tests/pnpm_workspace_groups_json_imports/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jbedard jbedard changed the title fix: ensure package target always contains package.json refactor: remove unnecessary unsafe slice modification Feb 14, 2026
@jbedard jbedard merged commit 704a9b6 into main Feb 14, 2026
3 checks passed
@jbedard jbedard deleted the shared-json branch February 14, 2026 02:42
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.

2 participants