Add endpoint URLs to the API specification (phase 1) (#3469) (#3524)

Author: swallez

compiler-rs/Cargo.lock

@@ -17,6 +17,7 @@ derive_more = "1.0.0-beta.6"
 either_n = "0.2"
 icu_segmenter = "1"
 indexmap = "2"
+itertools = "0.14"
 maplit = "1"
 once_cell = "1.16"
 openapiv3 = "2"

compiler-rs/Cargo.toml

@@ -11,6 +11,8 @@ serde_json = { workspace = true }
 once_cell = { workspace = true }
 anyhow = { workspace = true }
 indexmap = { workspace = true, features = ["serde"] }
+itertools = { workspace = true }
+
 
 arcstr = { workspace = true, features = ["serde", "substr"] }
 clap = { workspace = true, features = ["derive"] }

compiler-rs/clients_schema/Cargo.toml

@@ -0,0 +1,121 @@
+use std::collections::HashMap;
+use std::path::{Path, PathBuf};
+use clap::Parser;
+use itertools::Itertools;
+
+
+fn main() -> anyhow::Result<()> {
+    let cli = Cli::parse();
+    cli.run()?;
+    Ok(())
+}
+
+// Example usage:
+// (cd compiler-rs; find ../specification -name '*Request.ts' | cargo run --bin add_url_paths ../output/schema/schema.json | sh)
+
+/// Adds url paths to request definitions. Stdin must be a list of files, one per line.
+/// Outputs a shell script that uses ast-grep.
+#[derive(Debug, Parser)]
+#[command(author, version, about, long_about)]
+pub struct Cli {
+    /// input schema file, eg: ../output/schema/schema-no-generics.json
+    schema: PathBuf,
+}
+
+impl Cli {
+    pub fn run(&self) -> anyhow::Result<()> {
+
+        // Canonicalize all file names, so that we can do some suffix mapping from the schema locations.
+        let files: Vec<PathBuf> = std::io::read_to_string(std::io::stdin())?
+            .lines()
+            .flat_map(|line| std::fs::canonicalize(line)
+                .map_err(|e| {
+                    eprintln!("File {} not found", line);
+                    Result::<PathBuf, _>::Err(e)
+                })) // Remove errors
+            .collect();
+
+        let json = std::fs::read_to_string(&self.schema)?;
+        let schema = clients_schema::IndexedModel::from_reader(json.as_bytes())?;
+
+        let mut location_to_request = HashMap::<&Path, &clients_schema::Endpoint>::new();
+        for ep in &schema.endpoints {
+            let Some(req_name) = ep.request.as_ref() else {
+                //eprintln!("Skipping endpoint {} with no request", ep.name);
+                continue;
+            };
+
+            let type_def = schema.types.get(req_name).unwrap();
+            let location = type_def.base().spec_location.as_ref().unwrap();
+            let location = Path::new(location.split_once('#').unwrap().0);
+
+            location_to_request.insert(location, ep);
+        };
+
+        for file in files {
+            if let Some((_, endpoint)) = location_to_request.iter().find(|(location, _)| file.ends_with(location)) {
+                generate_astgrep_command(&file, endpoint);
+            } else {
+                eprintln!("No request found for {:?}", file);
+            }
+        }
+
+        Ok(())
+    }
+}
+
+fn generate_astgrep_command(file: &Path, endpoint: &clients_schema::Endpoint) {
+
+    let text = std::fs::read_to_string(file).unwrap();
+    if text.contains("urls:") {
+        eprintln!("Found an existing 'url' property. Skipping {file:?}");
+        return;
+    }
+
+    // We cannot express conditional parts in the source form of patterns.
+
+    // Requests with generic parameters
+    let request_expr = if text.contains("Request<") {
+        "Request<$$$PARAM>"
+    } else {
+        "Request"
+    };
+
+    // A handful of requests don't have an extends clause
+    let extends_expr = if text.contains(" extends ") {
+        "extends $REQBASE"
+    } else {
+        ""
+    };
+
+    let urls: String = endpoint.urls.iter().map(|url| {
+        let path = &url.path;
+        let methods = url.methods.iter().map(|method| format!("\"{}\"", method)).join(", ");
+        let deprecation = match &url.deprecation {
+            Some(deprecation) => format!("/** @deprecated {} {} */\n      ", deprecation.version, deprecation.description),
+            None => "".to_string(),
+        };
+
+        format!(r#"    {{
+      {deprecation}path: "{path}",
+      methods: [{methods}]
+    }}"#)
+    }).join(",\n");
+
+    let pattern = format!(r#"interface {request_expr} {extends_expr} {{
+  $$$PROPS
+}}"#);
+
+    let fix = format!(r#"interface {request_expr} {extends_expr} {{
+  urls: [
+{urls}
+  ],
+  $$$PROPS
+}}"#);
+
+    let file = file.to_str().unwrap();
+    println!("#----- {file}");
+    println!(r#"ast-grep --update-all --lang ts --pattern '{pattern}' --rewrite '{fix}' "{file}""#);
+
+    println!();
+}

compiler-rs/clients_schema/src/bin/add_url_paths.rs

@@ -53,7 +53,7 @@ import {
   verifyUniqueness,
   parseJsDocTags,
   deepEqual,
-  sourceLocation, sortTypeDefinitions
+  sourceLocation, sortTypeDefinitions, parseDeprecation
 } from './utils'
 
 const jsonSpec = buildJsonSpec()
@@ -210,14 +210,6 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int
       if (mapping == null) {
         throw new Error(`Cannot find url template for ${namespace}, very likely the specification folder does not follow the rest-api-spec`)
       }
-      // list of unique dynamic parameters
-      const urlTemplateParams = [...new Set(
-        mapping.urls.flatMap(url => url.path.split('/')
-          .filter(part => part.includes('{'))
-          .map(part => part.slice(1, -1))
-        )
-      )]
-      const methods = [...new Set(mapping.urls.flatMap(url => url.methods))]
 
       let pathMember: Node | null = null
       let bodyProperties: model.Property[] = []
@@ -226,39 +218,50 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int
 
       // collect path/query/body properties
       for (const member of declaration.getMembers()) {
-        // we are visiting `path_parts, `query_parameters` or `body`
+        // we are visiting `urls`, `path_parts, `query_parameters` or `body`
         assert(
           member,
           Node.isPropertyDeclaration(member) || Node.isPropertySignature(member),
           'Class and interfaces can only have property declarations or signatures'
         )
-        const property = visitRequestOrResponseProperty(member)
-        if (property.name === 'path_parts') {
+        const name = member.getName()
+        if (name === 'urls') {
+          // Overwrite the endpoint urls read from the json-rest-spec
+          // TODO: once all spec files are using it, make it mandatory.
+          mapping.urls = visitUrls(member)
+        } else if (name === 'path_parts') {
+          const property = visitRequestOrResponseProperty(member)
           assert(member, property.properties.length > 0, 'There is no need to declare an empty object path_parts, just remove the path_parts declaration.')
           pathMember = member
           type.path = property.properties
-        } else if (property.name === 'query_parameters') {
+        } else if (name === 'query_parameters') {
+          const property = visitRequestOrResponseProperty(member)
           assert(member, property.properties.length > 0, 'There is no need to declare an empty object query_parameters, just remove the query_parameters declaration.')
           type.query = property.properties
-        } else if (property.name === 'body') {
+        } else if (name === 'body') {
+          const property = visitRequestOrResponseProperty(member)
           bodyMember = member
-          assert(
-            member,
-            methods.some(method => ['POST', 'PUT', 'DELETE'].includes(method)),
-            `${namespace}.${name} can't have a body, allowed methods: ${methods.join(', ')}`
-          )
           if (property.valueOf != null) {
             bodyValue = property.valueOf
           } else {
             assert(member, property.properties.length > 0, 'There is no need to declare an empty object body, just remove the body declaration.')
             bodyProperties = property.properties
           }
         } else {
-          assert(member, false, `Unknown request property: ${property.name}`)
+          assert(member, false, `Unknown request property: ${name}`)
         }
       }
 
       // validate path properties
+      // list of unique dynamic parameters
+      const urlTemplateParams = [...new Set(
+        mapping.urls.flatMap(url => url.path.split('/')
+          .filter(part => part.includes('{'))
+          .map(part => part.slice(1, -1))
+        )
+      )]
+      const methods = [...new Set(mapping.urls.flatMap(url => url.methods))]
+
       for (const part of type.path) {
         assert(
           pathMember as Node,
@@ -282,6 +285,13 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int
       }
 
       // validate body
+      if (bodyMember != null) {
+        assert(
+          bodyMember,
+          methods.some(method => ['POST', 'PUT', 'DELETE'].includes(method)),
+            `${namespace}.${name} can't have a body, allowed methods: ${methods.join(', ')}`
+        )
+      }
       // the body can either be a value (eg Array<string> or an object with properties)
       if (bodyValue != null) {
         // Propagate required body value nature based on TS question token being present.
@@ -587,3 +597,80 @@ function visitRequestOrResponseProperty (member: PropertyDeclaration | PropertyS
 
   return { name, properties, valueOf }
 }
+
+/**
+ * Parse the 'urls' property of a request definition. Format is:
+ * ```
+ * urls: [
+ *   {
+ *     /** @deprecated 1.2.3 Use something else
+ *     path: '/some/path',
+ *     methods: ["GET", "POST"]
+ *   }
+ * ]
+ * ```
+ */
+function visitUrls (member: PropertyDeclaration | PropertySignature): model.UrlTemplate[] {
+  const value = member.getTypeNode()
+
+  // Literal arrays are exposed as tuples by ts-morph
+  assert(value, Node.isTupleTypeNode(value), '"urls" should be an array')
+
+  const result: model.UrlTemplate[] = []
+
+  value.forEachChild(urlNode => {
+    assert(urlNode, Node.isTypeLiteral(urlNode), '"urls" members should be objects')
+
+    const urlTemplate: any = {}
+
+    urlNode.forEachChild(node => {
+      assert(node, Node.isPropertySignature(node), "Expecting 'path' and 'methods' properties")
+
+      const name = node.getName()
+      const propValue = node.getTypeNode()
+
+      if (name === 'path') {
+        assert(propValue, Node.isLiteralTypeNode(propValue), '"path" should be a string')
+
+        const pathLit = propValue.getLiteral()
+        assert(pathLit, Node.isStringLiteral(pathLit), '"path" should be a string')
+
+        urlTemplate.path = pathLit.getLiteralValue()
+
+        // Deprecation
+        const jsDoc = node.getJsDocs()
+        const tags = parseJsDocTags(jsDoc)
+        const deprecation = parseDeprecation(tags, jsDoc)
+        if (deprecation != null) {
+          urlTemplate.deprecation = deprecation
+        }
+        if (Object.keys(tags).length > 0) {
+          assert(jsDoc, false, `Unknown annotations: ${Object.keys(tags).join(', ')}`)
+        }
+      } else if (name === 'methods') {
+        assert(propValue, Node.isTupleTypeNode(propValue), '"methods" should be an array')
+
+        const methods: string[] = []
+        propValue.forEachChild(node => {
+          assert(node, Node.isLiteralTypeNode(node), '"methods" should contain strings')
+
+          const nodeLit = node.getLiteral()
+          assert(nodeLit, Node.isStringLiteral(nodeLit), '"methods" should contain strings')
+
+          methods.push(nodeLit.getLiteralValue())
+        })
+        assert(node, methods.length > 0, "'methods' should not be empty")
+        urlTemplate.methods = methods
+      } else {
+        assert(node, false, "Expecting 'path' or 'methods'")
+      }
+    })
+
+    assert(urlTemplate, urlTemplate.path, "Missing required property 'path'")
+    assert(urlTemplate, urlTemplate.methods, "Missing required property 'methods'")
+
+    result.push(urlTemplate)
+  })
+
+  return result
+}

compiler/src/model/build-model.ts

@@ -576,12 +576,21 @@ export function modelProperty (declaration: PropertySignature | PropertyDeclarat
  * Pulls @deprecated from types and properties
  */
 function setDeprecated (type: model.BaseType | model.Property | model.EnumMember, tags: Record<string, string>, jsDocs: JSDoc[]): void {
+  const deprecation = parseDeprecation(tags, jsDocs)
+  if (deprecation != null) {
+    type.deprecation = deprecation
+  }
+}
+
+export function parseDeprecation (tags: Record<string, string>, jsDocs: JSDoc[]): model.Deprecation | undefined {
   if (tags.deprecated !== undefined) {
     const [version, ...description] = tags.deprecated.split(' ')
     assert(jsDocs, semver.valid(version), 'Invalid semver value')
-    type.deprecation = { version, description: description.join(' ') }
+    delete tags.deprecated
+    return { version, description: description.join(' ') }
+  } else {
+    return undefined
   }
-  delete tags.deprecated
 }
 
 /**