Skip to content

Commit 4d23798

Browse files
Halimaosdankel
andauthored
refactor: forc init generate unexpected project_name when project dir contains dot (#5455)
## 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: ![image](https://github.com/FuelLabs/sway/assets/25278203/b6073a20-81ea-45cb-9b58-842e43cb3257) #### After: ![image](https://github.com/FuelLabs/sway/assets/25278203/4c48fffd-8da6-473c-b738-1ab2721dd53b) ## 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*` or `New Feature` labels where relevant. - [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: Sophie Dankel <47993817+sdankel@users.noreply.github.com>
1 parent 29d5f9a commit 4d23798

File tree

6 files changed

+62
-12
lines changed

6 files changed

+62
-12
lines changed

forc-pkg/src/manifest/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ pub mod build_profile;
33
use crate::pkg::{manifest_file_missing, parsing_failed, wrong_program_type};
44
use anyhow::{anyhow, bail, Context, Result};
55
use forc_tracing::println_warning;
6-
use forc_util::validate_name;
6+
use forc_util::{validate_name, validate_project_name};
77
use serde::{Deserialize, Serialize};
88
use serde_with::{serde_as, DisplayFromStr};
99
use std::{
@@ -550,7 +550,7 @@ impl PackageManifest {
550550
/// 2. The validity of the details provided. Makes sure that there are no mismatching detail
551551
/// declarations (to prevent mixing details specific to certain types).
552552
pub fn validate(&self) -> Result<()> {
553-
validate_name(&self.project.name, "package name")?;
553+
validate_project_name(&self.project.name)?;
554554
if let Some(ref org) = self.project.organization {
555555
validate_name(org, "organization name")?;
556556
}

forc-util/src/lib.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,11 @@ pub fn lock_path(manifest_dir: &Path) -> PathBuf {
215215
manifest_dir.join(constants::LOCK_FILE_NAME)
216216
}
217217

218+
pub fn validate_project_name(name: &str) -> Result<()> {
219+
restricted::is_valid_project_name_format(name)?;
220+
validate_name(name, "project name")
221+
}
222+
218223
// Using (https://github.com/rust-lang/cargo/blob/489b66f2e458404a10d7824194d3ded94bc1f4e4/src/cargo/util/toml/mod.rs +
219224
// https://github.com/rust-lang/cargo/blob/489b66f2e458404a10d7824194d3ded94bc1f4e4/src/cargo/ops/cargo_new.rs) for reference
220225

@@ -223,17 +228,17 @@ pub fn validate_name(name: &str, use_case: &str) -> Result<()> {
223228
restricted::contains_invalid_char(name, use_case)?;
224229

225230
if restricted::is_keyword(name) {
226-
bail!("the name `{name}` cannot be used as a package name, it is a Sway keyword");
231+
bail!("the name `{name}` cannot be used as a {use_case}, it is a Sway keyword");
227232
}
228233
if restricted::is_conflicting_artifact_name(name) {
229234
bail!(
230-
"the name `{name}` cannot be used as a package name, \
235+
"the name `{name}` cannot be used as a {use_case}, \
231236
it conflicts with Forc's build directory names"
232237
);
233238
}
234239
if name.to_lowercase() == "test" {
235240
bail!(
236-
"the name `test` cannot be used as a project name, \
241+
"the name `test` cannot be used as a {use_case}, \
237242
it conflicts with Sway's built-in test library"
238243
);
239244
}

forc-util/src/restricted.rs

+45
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// This is based on https://github.com/rust-lang/cargo/blob/489b66f2e458404a10d7824194d3ded94bc1f4e4/src/cargo/util/restricted_names.rs
33

44
use anyhow::{bail, Result};
5+
use regex::Regex;
56
use std::path::Path;
67

78
/// Returns `true` if the name contains non-ASCII characters.
@@ -94,6 +95,18 @@ pub fn is_glob_pattern<T: AsRef<str>>(name: T) -> bool {
9495
name.as_ref().contains(&['*', '?', '[', ']'][..])
9596
}
9697

98+
/// Check the project name format.
99+
pub fn is_valid_project_name_format(name: &str) -> Result<()> {
100+
let re = Regex::new(r"^([a-zA-Z]([a-zA-Z0-9-_]+)|)$").unwrap();
101+
if !re.is_match(name) {
102+
bail!(
103+
"'{name}' is not a valid name for a project. \n\
104+
The name may use letters, numbers, hyphens, and underscores, and must start with a letter."
105+
);
106+
}
107+
Ok(())
108+
}
109+
97110
#[test]
98111
fn test_invalid_char() {
99112
assert_eq!(
@@ -130,3 +143,35 @@ fn test_invalid_char() {
130143
std::result::Result::Ok(())
131144
));
132145
}
146+
147+
#[test]
148+
fn test_is_valid_project_name_format() {
149+
let assert_valid = |name: &str| {
150+
is_valid_project_name_format(name).expect("this should pass");
151+
};
152+
153+
let assert_invalid = |name: &str, expected_error: &str| {
154+
assert_eq!(
155+
is_valid_project_name_format(name).map_err(|e| e.to_string()),
156+
Err(expected_error.into())
157+
);
158+
};
159+
160+
let format_error_message = |name: &str| -> String {
161+
format!(
162+
"'{}' is not a valid name for a project. \n\
163+
The name may use letters, numbers, hyphens, and underscores, and must start with a letter.",
164+
name
165+
)
166+
};
167+
168+
// Test valid project names
169+
assert_valid("mock_project_name");
170+
assert_valid("mock_project_name123");
171+
assert_valid("mock_project_name-123-_");
172+
173+
// Test invalid project names
174+
assert_invalid("1mock_project", &format_error_message("1mock_project"));
175+
assert_invalid("mock_.project", &format_error_message("mock_.project"));
176+
assert_invalid("mock_/project", &format_error_message("mock_/project"));
177+
}

forc/src/cli/commands/new.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::{cli::init::Command as InitCommand, ops::forc_init::init};
22
use anyhow::anyhow;
33
use clap::Parser;
4-
use forc_util::{forc_result_bail, validate_name, ForcResult};
4+
use forc_util::{forc_result_bail, validate_project_name, ForcResult};
55
use std::path::{Path, PathBuf};
66

77
forc_util::cli_examples! {
@@ -55,7 +55,7 @@ pub(crate) fn exec(command: Command) -> ForcResult<()> {
5555
} = command;
5656

5757
match &name {
58-
Some(name) => validate_name(name, "project name")?,
58+
Some(name) => validate_project_name(name)?,
5959
None => {
6060
// If there is no name specified for the project, the last component of the `path` (directory name)
6161
// will be used by default so we should also check that.
@@ -64,7 +64,7 @@ pub(crate) fn exec(command: Command) -> ForcResult<()> {
6464
.file_name()
6565
.ok_or_else(|| anyhow!("missing path for new command"))?
6666
.to_string_lossy();
67-
validate_name(&directory_name, "project_name")?;
67+
validate_project_name(&directory_name)?;
6868
}
6969
}
7070

forc/src/ops/forc_init.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::cli::InitCommand;
22
use crate::utils::{defaults, program_type::ProgramType};
33
use anyhow::Context;
4-
use forc_util::{forc_result_bail, validate_name, ForcResult};
4+
use forc_util::{forc_result_bail, validate_project_name, ForcResult};
55
use std::fs;
66
use std::io::Write;
77
use std::path::{Path, PathBuf};
@@ -78,7 +78,7 @@ pub fn init(command: InitCommand) -> ForcResult<()> {
7878
.into_owned(),
7979
};
8080

81-
validate_name(&project_name, "project name")?;
81+
validate_project_name(&project_name)?;
8282

8383
let init_type = match (
8484
command.contract,

forc/src/ops/forc_template.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use forc_pkg::{
44
manifest::{self, PackageManifest},
55
source::{self, git::Url},
66
};
7-
use forc_util::validate_name;
7+
use forc_util::validate_project_name;
88
use fs_extra::dir::{copy, CopyOptions};
99
use std::fs::File;
1010
use std::io::{Read, Write};
@@ -14,7 +14,7 @@ use sway_utils::constants;
1414
use tracing::info;
1515

1616
pub fn init(command: TemplateCommand) -> Result<()> {
17-
validate_name(&command.project_name, "project name")?;
17+
validate_project_name(&command.project_name)?;
1818
// The name used for the temporary local repo directory used for fetching the template.
1919
let local_repo_name = command
2020
.template_name

0 commit comments

Comments
 (0)