sway
sway copied to clipboard
refactor: `forc init` generate unexpected project_name when project dir contains dot
Description
Refer #5434
Found this issue when I tried to run forc init --script
under a directory(/tmp/path_with_._dot
) that contains a dot ".". The value of project.name
in the generated file Forc.toml
is truncated by the dot "." as path_with_
.
Since project_dir
must be a directory(has checked before), we can use file_name
instead of file_stem
to get the last-level dir name. Also, I think it would be better to replace "." with "_", otherwise a project_name
that contains .
would fail with validation validate_name(&project_name, "project name")?;
Screenshots
Before:
After:
Checklist
- [ ] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand areas.
- [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book).
- [ ] I have added tests that prove my fix is effective or that my feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
Breaking*
orNew Feature
labels where relevant. - [ ] I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
- [ ] I have requested a review from the relevant team or maintainers.
Thanks for the PR! I will test this locally but it seems like this will allow project with
.
in the name, which is not something we should be doing imo. Blocking for now so that I can test
The .
will be replaced with _
. This pr just changed the value of project_name
by using path_with___dot
instead of path_with_
. See the screenshot below:
I think we should disallow project names with dots or any special character other than _
, similar to cargo. Numbers should be allowed but not as a first digit.
Here's a regex we can use: ^([a-zA-Z]([a-zA-Z0-9-_]+)|)$
I think we should disallow project names with dots or any special character other than
_
, similar to cargo. Numbers should be allowed but not as a first digit.Here's a regex we can use:
^([a-zA-Z]([a-zA-Z0-9-_]+)|)$
If I understood correctly, the suggested changes are to revert the logic of getting project_name and add a new regex ^([a-zA-Z]([a-zA-Z0-9-_]+)|)$
checker into the function validate_name
?
I think we should disallow project names with dots or any special character other than
_
, similar to cargo. Numbers should be allowed but not as a first digit. Here's a regex we can use:^([a-zA-Z]([a-zA-Z0-9-_]+)|)$
If I understood correctly, the suggested changes are to revert the logic of getting project_name and add a new regex
^([a-zA-Z]([a-zA-Z0-9-_]+)|)$
checker into the functionvalidate_name
?
Yes, I think that's the best path forward. If it doesn't match the regex, we can show an error like "The project name can be a combination of letters, numbers and underscores, and must start with a letter."
Yes, I think that's the best path forward. If it doesn't match the regex, we can show an error like "The project name can be a combination of letters, numbers, and underscores, and must start with a letter."
- The function
validate_name
is not only used forproject name
but also fororganization name
andpackage name
- The param
use_case
is not an enum, there may exist different values for the similar caseproject name
, and it's hard to judge whether the case isproject name
or not
@sdankel For the reason above, I think we should add a new function validate_project_name
like this:
pub fn validate_project_name(name: &str) -> Result<()> {
let re = Regex::new(r"^([a-zA-Z]([a-zA-Z0-9-_]+)|)$").unwrap();
if !re.is_match(name) {
bail!(
"the project name `{name}` cannot be used as a project name.\n\
project name can be a combination of letters, numbers, hyphen, and underscores, \
and must start with a letter."
);
}
validate_name(name, "project name")
}
BTW, is the '-' hyphen valid in the project name?
BTW, is the '-' hyphen valid in the project name?
Yes, good catch! -
should be allowed.
The function validate_name is not only used for project name but also for organization name and package name
I don't think there is a difference between package name and project name. I agree that organization name should not have this validation.
BTW, is the '-' hyphen valid in the project name?
Yes, good catch!
-
should be allowed.The function validate_name is not only used for project name but also for organization name and package name
I don't think there is a difference between package name and project name. I agree that organization name should not have this validation.
Suggestion changes done~
Can we get this merged now? Is there anything else that needs to be added?