refactor for loops (#66)

.github/workflows/presubmit.yml

@@ -50,6 +50,7 @@ jobs:
           N2S_NDK_PATH: ${{ env.android }}
         run: |
           set -x
+          mkdir -p "${{ env.android }}"
           PATH="${{ env.android }}/depot_tools:${PATH}" cargo run --release -- --skip-gen-ninja --angle-path "${{ env.android }}/external/angle" --aosp-path "${{ env.android }}"
       - name: Check generated files
         shell: bash

src/context.rs

@@ -26,18 +26,18 @@ pub struct Context {
 
 impl Context {
     fn help(&self, projects: &Vec<&mut dyn Project>) -> String {
-        let mut projects_help = String::new();
-        for project in projects {
-            projects_help += "  ";
-            projects_help += project.get_id().str();
-            projects_help += "\n";
-        }
+        let projects_help = projects
+            .iter()
+            .map(|project| project.get_id().str())
+            .collect::<Vec<&str>>()
+            .join("\n  ");
         format!(
             "
 USAGE: {0} [OPTIONS] [PROJECTS]
 
 PROJECTS:
-{projects_help}
+  {projects_help}
+
 OPTIONS:
   {ANGLE_PATH} <path>  Path to angle source repository (required only for the `angle` project)
   {AOSP_PATH} <path>   Path to Android tree (required for most project)

src/ninja_target/common.rs

@@ -28,27 +28,21 @@ pub fn get_link_libraries(libs: &str) -> Result<(Vec<PathBuf>, Vec<PathBuf>), St
     Ok((static_libraries, shared_libraries))
 }
 
-pub fn get_defines(defs: &str) -> Vec<String> {
-    let mut defines = Vec::new();
-    for define in defs.split("-D") {
-        if define.is_empty() {
-            continue;
-        }
-        defines.push(define.trim().replace("\\(", "(").replace("\\)", ")"));
-    }
+pub fn get_defines(defines: &str) -> Vec<String> {
     defines
+        .split("-D")
+        .filter(|define| !define.is_empty())
+        .map(|define| define.trim().replace("\\(", "(").replace("\\)", ")"))
+        .collect()
 }
 
-pub fn get_includes(incs: &str, build_path: &Path) -> Vec<PathBuf> {
-    let mut includes = Vec::new();
-    for inc in incs.split(" ") {
-        let include = inc.strip_prefix("-I").unwrap_or(inc);
-        if include.is_empty() || include == "isystem" {
-            continue;
-        }
-        includes.push(canonicalize_path(include, build_path));
-    }
+pub fn get_includes(includes: &str, build_path: &Path) -> Vec<PathBuf> {
     includes
+        .split(" ")
+        .map(|include| include.strip_prefix("-I").unwrap_or(include))
+        .filter(|include| !(include.is_empty() || *include == "isystem"))
+        .map(|include| canonicalize_path(include, build_path))
+        .collect()
 }
 
 pub fn get_link_flags(flags: &str) -> (Option<PathBuf>, Vec<String>) {
@@ -64,20 +58,16 @@ pub fn get_link_flags(flags: &str) -> (Option<PathBuf>, Vec<String>) {
 }
 
 pub fn get_cflags(flags: &str) -> Vec<String> {
-    let mut cflags = Vec::new();
-    for flag in flags.split(" ") {
-        if flag.is_empty() {
-            continue;
-        }
-        cflags.push(flag.to_string());
-    }
-    cflags
+    flags
+        .split(" ")
+        .filter(|flag| !flag.is_empty())
+        .map(|flag| flag.to_string())
+        .collect()
 }
 
-pub fn get_sources(ins: &Vec<PathBuf>, build_path: &Path) -> Vec<PathBuf> {
-    let mut inputs = Vec::new();
-    for input in ins {
-        inputs.push(canonicalize_path(input, build_path));
-    }
+pub fn get_sources(inputs: &Vec<PathBuf>, build_path: &Path) -> Vec<PathBuf> {
     inputs
+        .into_iter()
+        .map(|input| canonicalize_path(input, build_path))
+        .collect()
 }

src/parser.rs

@@ -11,10 +11,10 @@ fn parse_output_section(section: &str) -> Result<(Vec<PathBuf>, Vec<PathBuf>), S
     if split.clone().count() < 1 {
         return error!("parse_output_section failed: '{section}'");
     }
-    let mut outputs = Vec::new();
-    for output in split.nth(0).unwrap().trim().split(" ") {
-        outputs.push(PathBuf::from(output.trim()));
-    }
+    let split_outputs = split.nth(0).unwrap().trim().split(" ");
+    let outputs = split_outputs
+        .map(|output| PathBuf::from(output.trim()))
+        .collect();
     let mut implicit_outputs = Vec::new();
     if let Some(implicit_outs) = split.next() {
         for implicit_output in implicit_outs.trim().split(" ") {
@@ -30,10 +30,9 @@ fn parse_input_and_rule_section(section: &str) -> Result<(String, Vec<PathBuf>),
         return error!("parse_input_and_rule_section failed: '{section}'");
     }
     let rule = String::from(split.nth(0).unwrap());
-    let mut inputs = Vec::new();
-    for input in split {
-        inputs.push(PathBuf::from(input.trim()));
-    }
+    let inputs = split
+        .map(|input| PathBuf::from(input.trim()))
+        .collect::<Vec<PathBuf>>();
     Ok((rule, inputs))
 }
 

src/project/angle.rs

@@ -100,10 +100,10 @@ impl Project for Angle {
             "SPDX-license-identifier-Apache-2.0",
             "LICENSE",
         );
-        let mut targets_to_generate = Vec::new();
-        for target in TARGETS {
-            targets_to_generate.push(PathBuf::from(target));
-        }
+        let targets_to_generate = TARGETS
+            .into_iter()
+            .map(|target| PathBuf::from(target))
+            .collect();
         package.generate(targets_to_generate, targets, self)?;
 
         Ok(package)

src/project/clspv.rs

@@ -103,12 +103,12 @@ impl Project for Clspv {
         let mut deps: GenDepsMap = HashMap::new();
         match project {
             ProjectId::SpirvHeaders => {
-                let mut files = Vec::new();
-                for dep in &self.gen_deps {
-                    if dep.starts_with(&self.spirv_headers_path) {
-                        files.push(dep.clone());
-                    }
-                }
+                let files = self
+                    .gen_deps
+                    .iter()
+                    .filter(|dep| dep.starts_with(&self.spirv_headers_path))
+                    .map(|dep| dep.clone())
+                    .collect();
                 deps.insert(GenDeps::SpirvHeaders, files);
             }
             ProjectId::LlvmProject => {

src/project/llvm_project.rs

@@ -80,9 +80,7 @@ impl Project for LlvmProject {
             "tools/clang/include/clang/Basic/Version.inc",
             "tools/clang/include/clang/Config/config.h",
         ];
-        for header in missing_gen_deps {
-            gen_deps.insert(PathBuf::from(header));
-        }
+        gen_deps.extend(missing_gen_deps.iter().map(|dep| PathBuf::from(dep)));
 
         let mut gen_deps_folders: HashSet<PathBuf> = HashSet::new();
         for gen_dep in &gen_deps {
@@ -201,10 +199,7 @@ impl Project for LlvmProject {
                 "-DBLAKE3_NO_SSE2",
             ]);
         }
-        cflags.iter().fold(Vec::new(), |mut vec, flag| {
-            vec.push(flag.to_string());
-            vec
-        })
+        cflags.iter().map(|flag| flag.to_string()).collect()
     }
 
     fn get_include(&self, include: &Path) -> PathBuf {

src/soong_module.rs

@@ -20,12 +20,8 @@ impl CcLibraryHeaders {
     }
 }
 
-fn print_indent(level: u8) -> String {
-    let mut indent = String::new();
-    for _ in 0..level {
-        indent += "    ";
-    }
-    indent
+fn print_indent(level: usize) -> String {
+    "    ".repeat(level)
 }
 
 #[derive(Debug)]
@@ -54,7 +50,7 @@ impl SoongNamedProp {
         }
     }
 
-    fn print(self, indent_level: u8) -> String {
+    fn print(self, indent_level: usize) -> String {
         let mut output = String::new();
         output += &format!("{0}{1}: ", print_indent(indent_level), self.name);
         output += &(match self.prop {

src/soong_package.rs

@@ -8,43 +8,27 @@ use crate::project::Project;
 use crate::soong_module::*;
 use crate::utils::*;
 
-fn update_cflags_with_defines(
-    defines: Vec<String>,
-    project: &dyn Project,
-    cflags: &mut HashSet<String>,
-) {
-    for define in defines {
-        if project.ignore_define(&define) {
-            continue;
-        }
-        cflags.insert("-D".to_string() + &project.get_define(&define));
-    }
+fn update_cflags_with_defines(defines: Vec<String>, project: &dyn Project) -> Vec<String> {
+    defines
+        .iter()
+        .filter(|def| !project.ignore_define(def))
+        .map(|def| "-D".to_string() + &project.get_define(def))
+        .collect()
 }
 
-fn update_cflags(cflags: Vec<String>, project: &dyn Project, all_cflags: &mut HashSet<String>) {
-    for cflag in cflags {
-        if project.ignore_cflag(&cflag) {
-            continue;
-        }
-        all_cflags.insert(cflag);
-    }
+fn update_cflags(cflags: Vec<String>, project: &dyn Project) -> Vec<String> {
+    cflags
+        .into_iter()
+        .filter(|cflag| !project.ignore_cflag(cflag))
+        .collect()
 }
 
-fn update_includes(
-    incs: Vec<PathBuf>,
-    project: &dyn Project,
-    includes: &mut HashSet<String>,
-    src_path: &Path,
-) {
-    for include in incs {
-        if project.ignore_include(&include) {
-            continue;
-        }
-        includes.insert(path_to_string(strip_prefix(
-            project.get_include(&include),
-            src_path,
-        )));
-    }
+fn update_includes(includes: Vec<PathBuf>, project: &dyn Project, src_path: &Path) -> Vec<String> {
+    includes
+        .into_iter()
+        .filter(|include| !project.ignore_include(include))
+        .map(|inc| path_to_string(strip_prefix(project.get_include(&inc), src_path)))
+        .collect()
 }
 
 #[derive(Debug)]
@@ -184,32 +168,28 @@ impl<'a> SoongPackage<'a> {
             static_libs.extend(static_libraries);
             shared_libs.extend(shared_libraries);
 
-            update_includes(
+            includes.extend(update_includes(
                 target.get_includes(self.build_path),
                 project,
-                &mut includes,
                 self.src_path,
-            );
-            update_cflags_with_defines(target.get_defines(), project, &mut cflags);
-            update_cflags(target.get_cflags(), project, &mut cflags);
+            ));
+            cflags.extend(update_cflags_with_defines(target.get_defines(), project));
+            cflags.extend(update_cflags(target.get_cflags(), project));
         }
 
-        update_includes(
+        includes.extend(update_includes(
             target.get_includes(self.build_path),
             project,
-            &mut includes,
             self.src_path,
-        );
-        update_cflags_with_defines(target.get_defines(), project, &mut cflags);
-        update_cflags(target.get_cflags(), project, &mut cflags);
+        ));
+        cflags.extend(update_cflags_with_defines(target.get_defines(), project));
+        cflags.extend(update_cflags(target.get_cflags(), project));
 
         let (version_script, link_flags) = target.get_link_flags();
-        let link_flags = link_flags.into_iter().fold(Vec::new(), |mut vec, flag| {
-            if !project.ignore_link_flag(&flag) {
-                vec.push(flag)
-            }
-            vec
-        });
+        let link_flags = link_flags
+            .into_iter()
+            .filter(|flag| !project.ignore_link_flag(flag))
+            .collect::<Vec<String>>();
         let (static_libraries, shared_libraries) = target.get_link_libraries()?;
         static_libs.extend(static_libraries);
         shared_libs.extend(shared_libraries);
@@ -376,20 +356,18 @@ impl<'a> SoongPackage<'a> {
         }
         if let Some((rsp_file, rsp_content)) = rule_cmd.1 {
             let rsp = "$(genDir)/".to_string() + &rsp_file;
-            let mut rsp_files = Vec::new();
-            for file in rsp_content.split(" ") {
-                if file.is_empty() {
-                    continue;
-                }
-                rsp_files.push(
+            let rsp_files = rsp_content
+                .split(" ")
+                .filter(|file| !file.is_empty())
+                .map(|file| {
                     String::from("$(location ")
                         + &path_to_string(strip_prefix(
                             canonicalize_path(file, self.build_path),
                             self.src_path,
                         ))
-                        + ")",
-                );
-            }
+                        + ")"
+                })
+                .collect::<Vec<String>>();
             cmd = "echo \\\"".to_string() + &rsp_files.join(" ") + "\\\" > " + &rsp + " && " + &cmd;
             cmd = cmd.replace("${rspfile}", &rsp);
         }

src/utils.rs

@@ -106,8 +106,14 @@ pub fn canonicalize_path<P: AsRef<Path>>(path: P, build_path: &Path) -> PathBuf
     } else {
         build_path
             .join(&path_buf)
-            .canonicalize()
-            .unwrap_or(path_buf)
+            .components()
+            .fold(PathBuf::new(), |path, component| {
+                if component == std::path::Component::ParentDir {
+                    path.parent().unwrap().to_path_buf()
+                } else {
+                    path.join(component)
+                }
+            })
     }
 }
 
@@ -155,7 +161,10 @@ pub fn split_path(path: &Path, delimiter: &str) -> Option<(PathBuf, PathBuf)> {
 }
 
 pub fn dep_name<P: AsRef<Path>>(from: &Path, prefix: P, path: &str, build_path: &Path) -> String {
-    path_to_id(Path::new(path).join(strip_prefix(canonicalize_path(from, build_path), prefix)))
+    path_to_id(Path::new(path).join(strip_prefix(
+        canonicalize_path(from, build_path),
+        canonicalize_path(prefix, build_path),
+    )))
 }
 
 pub fn copy_file(from: &Path, to: &Path) -> Result<(), String> {