fix: Validate project name as a Bazel module name during bootstrap - #144
fix: Validate project name as a Bazel module name during bootstrap#144ecoreng wants to merge 3 commits into
Conversation
`valdi bootstrap` accepted project names that Bazel later rejected. The
project name is used for both Bazel target/directory names, which are
case-sensitive, and the `module(name = ...)` value in MODULE.bazel, which
Bazel requires to be lowercase, to begin with a letter and to end with a
letter or digit. `sanitizeProjectName` intentionally preserves case for the
former, and `validateProjectName` only warned when sanitizing changed the
input, so a name containing uppercase letters passed every check.
Bazel was then the first thing to apply its own rule, during the projectsync
step at the very end of bootstrap:
Error in module: invalid module name 'MyProject': valid names must
1) only contain lowercase letters (a-z), digits (0-9), dots (.),
hyphens (-), and underscores (_); ...
By that point MODULE.bazel and the rest of the project had been written, so
re-running bootstrap failed with "Detected existing project files" instead.
Check the sanitized name against Bazel's module name rules in
`validateProjectName`, so the name is rejected at the prompt, before any
files are created, with a suggested valid alternative. The name is not
rewritten automatically: target and directory names stay case-sensitive and
the choice of name stays with the user.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
If we really consider this an error it should fail quickly like here. If we want to "sanitize" it we can but then we're assuming it's ok to change iMac to imac and it may not be brand friendly or whatever. Regardless there is a |
|
|
||
| // The name is used as the Bazel module name in MODULE.bazel, so reject anything | ||
| // Bazel would refuse before any files are written. | ||
| if (!isValidBazelModuleName(sanitized)) { |
There was a problem hiding this comment.
It looks like this Bazel-specific validation is being added to the shared validateProjectName(), which is also used by valdi new_module in newModule.ts:163
I double-checked the existing smoke-test name testNewModule: the new regex rejects it, so the command throws before creating the module. CamelCase is valid for these directory and BUILD target names because they do not become the root module(name = ...).
Would it make sense to apply isValidBazelModuleName() only in the bootstrap path, or split project and Valdi-module validation?
`validateProjectName` is shared by `valdi bootstrap` and `valdi new_module`.
The Bazel module name check only matters for bootstrap, where the name
becomes `module(name = ...)` in MODULE.bazel. Module names created by
`new_module` are case-sensitive directory/BUILD target names, so the check
wrongly rejected names like `testNewModule` there.
Make the check opt-in via `{ bazelModule: true }` and enable it only in the
bootstrap paths. `new_module` behavior is unchanged from before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem
valdi bootstrapaccepts a project name likeMyProject, then fails at the very end, after the project files are already written:The name becomes
module(name = ...)inMODULE.bazel, which Bazel requires to be lowercase. Nothing checks that until Bazel runs. Re-running bootstrap then fails withDetected existing project files, so you have to clean up by hand first (or the cleanup version)Fix
Check the name against Bazel's rules at the prompt, before any files are created:
The name is not corrected automatically — directory and target names stay case-sensitive, so that stays the user's choice.
Note:
123projectis now rejected instead of being turned into_123project.