Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve app name validation #853

Merged
merged 6 commits into from
Oct 20, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions loco-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

[package]
name = "loco-cli"
version = "0.2.9"
version = "0.2.10"
edition = "2021"
description = "loco cli website generator"
license = "Apache-2.0"
Expand Down Expand Up @@ -39,7 +39,6 @@ dialoguer = "0.11.0"
regex = { version = "1.10.2" }
ignore = "0.4.21"
fs_extra = "1.3.0"
lazy_static = "1.4.0"
tracing = "0.1.40"
fs-err = "2.11.0"
tracing-subscriber = { version = "0.3.16", features = ["env-filter"] }
Expand All @@ -48,6 +47,7 @@ cfg-if = "1.0.0"
dunce = { version = "1.0.4" }
strum = { version = "0.26", features = ["derive"] }
colored = "2"
unicode-xid = "0.2.6"
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we can drop checking for unicode by the fact that we don't allow unicode in app name.
its reasonable to just say app names are alphanumeric (ASCII)


[dev-dependencies]
trycmd = "0.14.19"
Expand Down
43 changes: 29 additions & 14 deletions loco-cli/src/prompt.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use std::{collections::BTreeMap, env};

use dialoguer::{theme::ColorfulTheme, Confirm, Input, Select};
use lazy_static::lazy_static;
use regex::Regex;
use strum::IntoEnumIterator;

use crate::{
Expand All @@ -11,10 +9,6 @@ use crate::{
Error,
};

lazy_static! {
static ref VALIDATE_APP_NAME: Regex = Regex::new(r"^[a-zA-Z0-9_]+$").unwrap();
}

/// Prompts the user to enter a valid application name for use with the Loco app
/// generator.
///
Expand All @@ -27,13 +21,19 @@ lazy_static! {
/// when could not prompt the question to the user or enter value is empty
pub fn app_name(name: Option<String>) -> crate::Result<String> {
if let Some(app_name) = env::var(env_vars::APP_NAME).ok().or(name) {
validate_app_name(app_name.as_str()).map_err(Error::msg)?;
validate_app_name(app_name.as_str()).map_err(|err| Error::msg(err.to_string()))?;
Ok(app_name)
} else {
let res = Input::with_theme(&ColorfulTheme::default())
.with_prompt("❯ App name?")
.default("myapp".into())
.validate_with(|input: &String| validate_app_name(input))
.validate_with(|input: &String| {
if let Err(err) = validate_app_name(input) {
Err(err.to_string())
} else {
Ok(())
}
})
.interact_text()?;
Ok(res)
}
Expand Down Expand Up @@ -116,14 +116,29 @@ pub fn warn_if_in_git_repo() -> crate::Result<()> {
/// characters to ensure compatibility with Cargo, the Rust package manager.
/// This function checks whether the provided application name complies with
/// these conventions.
fn validate_app_name(app: &str) -> Result<(), String> {
if !VALIDATE_APP_NAME.is_match(app) {
return Err(
"app name is invalid, illegal characters. keep names simple: myapp or my_app"
.to_owned(),
);
fn validate_app_name(app_name: &str) -> Result<(), &str> {
if app_name.is_empty() {
return Err("app name could not be empty");
}

let mut chars = app_name.chars();
if let Some(ch) = chars.next() {
if ch.is_ascii_digit() {
return Err("the name cannot start with a digit");
}
if !(unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_') {
return Err(
"the first character must be a Unicode XID start character (most letters or `_`)",
);
}
}
for ch in chars {
if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-') {
return Err(
"characters must be Unicode XID characters (numbers, `-`, `_`, or most letters)",
);
}
}
Ok(())
}

Expand Down
Loading