feat(js): Add ability for include in JS sourcemap upload options to be an object

js/index.d.ts

@@ -49,12 +49,19 @@ declare module '@sentry/cli' {
     silent?: boolean;
   }
 
+  /**
+   * Custom upload-sourcemaps options for a particular `include` path. In this
+   * case `paths` takes the place of `include` in the options so as to make it
+   * clear that this is not recursive.
+   */
+  export type SourceMapsPathDescriptor = Omit<SentryCliUploadSourceMapsOptions, 'include'> & { paths: string[] }
+
   export interface SentryCliUploadSourceMapsOptions {
     /**
      * One or more paths that Sentry CLI should scan recursively for sources.
      * It will upload all .map files and match associated .js files.
      */
-    include: string | string[];
+    include: Array<string | SourceMapsPathDescriptor>;
     /**
      * One or more paths to ignore during upload. Overrides entries in ignoreFile file.
      */

js/releases/__tests__/index.test.js

@@ -94,6 +94,73 @@ describe('SentryCli releases', () => {
           { silent: false }
         );
       });
+
+      test('handles multiple include entries', async () => {
+        expect.assertions(3);
+
+        const paths = ['path', 'other-path'];
+        await cli.releases.uploadSourceMaps('my-version', { include: paths });
+
+        expect(mockExecute).toHaveBeenCalledTimes(2);
+        paths.forEach(path =>
+          expect(mockExecute).toHaveBeenCalledWith(
+            [
+              'releases',
+              'files',
+              'my-version',
+              'upload-sourcemaps',
+              path,
+              '--ignore',
+              'node_modules',
+            ],
+            true,
+            false,
+            undefined,
+            { silent: false }
+          )
+        );
+      });
+
+      test('handles object-type include entries', async () => {
+        expect.assertions(3);
+
+        const paths = [{ paths: ['some-path'], ignore: ['not-me'] }, 'other-path'];
+        await cli.releases.uploadSourceMaps('my-version', { include: paths });
+
+        expect(mockExecute).toHaveBeenCalledTimes(2);
+
+        expect(mockExecute).toHaveBeenCalledWith(
+          [
+            'releases',
+            'files',
+            'my-version',
+            'upload-sourcemaps',
+            'some-path',
+            '--ignore',
+            'not-me', // note how this has been overridden
+          ],
+          true,
+          false,
+          undefined,
+          { silent: false }
+        );
+
+        expect(mockExecute).toHaveBeenCalledWith(
+          [
+            'releases',
+            'files',
+            'my-version',
+            'upload-sourcemaps',
+            'other-path',
+            '--ignore',
+            'node_modules',
+          ],
+          true,
+          false,
+          undefined,
+          { silent: false }
+        );
+      });
     });
   });
 });

js/releases/index.js

@@ -160,23 +160,58 @@ class Releases {
    * @memberof SentryReleases
    */
   uploadSourceMaps(release, options) {
-    if (!options || !options.include) {
-      throw new Error('options.include must be a vaild path(s)');
+    if (!options || !options.include || !Array.isArray(options.include)) {
+      throw new Error(
+        '`options.include` must be a vaild array of paths and/or path descriptor objects.'
+      );
     }
 
-    const uploads = options.include.map(sourcemapPath => {
-      const newOptions = { ...options };
+    // Each entry in the `include` array will map to an array of promises, which
+    // will in turn contain one promise per literal path value. Thus `uploads`
+    // will be an array of Promise arrays, which we'll flatten later.
+    const uploads = options.include.map(includeEntry => {
+      let pathOptions;
+      let uploadPaths;
+
+      if (typeof includeEntry === 'object') {
+        pathOptions = includeEntry;
+        uploadPaths = includeEntry.paths;
+
+        if (!Array.isArray(uploadPaths)) {
+          throw new Error(
+            `Path descriptor objects in \`options.include\` must contain a \`paths\` array. Got ${includeEntry}.`
+          );
+        }
+      }
+      // `includeEntry` should be a string, which we can wrap in an array to
+      // match the `paths` property in the descriptor object type
+      else {
+        pathOptions = {};
+        uploadPaths = [includeEntry];
+      }
+
+      const newOptions = { ...options, ...pathOptions };
       if (!newOptions.ignoreFile && !newOptions.ignore) {
         newOptions.ignore = DEFAULT_IGNORE;
       }
 
+      // args which apply to the entire `include` entry (everything besides the path)
       const args = ['releases']
         .concat(helper.getProjectFlagsFromOptions(options))
-        .concat(['files', release, 'upload-sourcemaps', sourcemapPath]);
-      return this.execute(helper.prepareCommand(args, SOURCEMAPS_SCHEMA, newOptions), true);
+        .concat(['files', release, 'upload-sourcemaps']);
+
+      return uploadPaths.map(path =>
+        // `execute()` is async and thus we're returning a promise here
+        this.execute(helper.prepareCommand([...args, path], SOURCEMAPS_SCHEMA, newOptions), true)
+      );
     });
 
-    return Promise.all(uploads);
+    // `uploads` is an array of Promise arrays, which needs to be flattened
+    // before being passed to `Promise.all()`. (`Array.flat()` doesn't exist in
+    // Node < 11; this polyfill takes advantage of the fact that `concat()` is
+    // willing to accept an arbitrary number of items to add to and/or iterables
+    // to unpack into the given array.)
+    return Promise.all([].concat(...uploads));
   }
 
   /**

src/api.rs

@@ -451,15 +451,15 @@ impl Api {
 
     /// Convenience method that downloads a file into the given file object.
     pub fn download(&self, url: &str, dst: &mut File) -> ApiResult<ApiResponse> {
-        self.request(Method::Get, &url)?
+        self.request(Method::Get, url)?
             .follow_location(true)?
             .send_into(dst)
     }
 
     /// Convenience method that downloads a file into the given file object
     /// and show a progress bar
     pub fn download_with_progress(&self, url: &str, dst: &mut File) -> ApiResult<ApiResponse> {
-        self.request(Method::Get, &url)?
+        self.request(Method::Get, url)?
             .follow_location(true)?
             .progress_bar_mode(ProgressBarMode::Response)?
             .send_into(dst)
@@ -470,7 +470,7 @@ impl Api {
     pub fn wait_until_available(&self, url: &str, duration: Duration) -> ApiResult<bool> {
         let started = Utc::now();
         loop {
-            match self.request(Method::Get, &url)?.send() {
+            match self.request(Method::Get, url)?.send() {
                 Ok(_) => return Ok(true),
                 Err(err) => {
                     if err.kind() != ApiErrorKind::RequestFailed {
@@ -1304,7 +1304,7 @@ impl Api {
         checkin: &CreateMonitorCheckIn,
     ) -> ApiResult<MonitorCheckIn> {
         let path = &format!("/monitors/{}/checkins/", PathArg(monitor),);
-        let resp = self.post(&path, checkin)?;
+        let resp = self.post(path, checkin)?;
         if resp.status() == 404 {
             return Err(ApiErrorKind::ResourceNotFound.into());
         }
@@ -1323,7 +1323,7 @@ impl Api {
             PathArg(monitor),
             PathArg(checkin_id),
         );
-        let resp = self.put(&path, checkin)?;
+        let resp = self.put(path, checkin)?;
         if resp.status() == 404 {
             return Err(ApiErrorKind::ResourceNotFound.into());
         }
@@ -1557,7 +1557,7 @@ impl ApiRequest {
             Method::Delete => handle.custom_request("DELETE")?,
         }
 
-        handle.url(&url)?;
+        handle.url(url)?;
 
         let request = ApiRequest {
             handle,

src/commands/difutil_find.rs

@@ -282,7 +282,7 @@ fn find_ids(
             eprintln!();
             eprintln!("missing debug information files:");
             for id in &remaining {
-                eprintln!("  {} ({})", id, id_hint(&id),);
+                eprintln!("  {} ({})", id, id_hint(id),);
             }
         }
     }

src/commands/login.rs

@@ -33,7 +33,7 @@ pub fn execute(matches: &ArgMatches<'_>) -> Result<(), Error> {
     println!();
     println!(
         "Sentry server: {}",
-        Url::parse(&config.get_base_url()?)?
+        Url::parse(config.get_base_url()?)?
             .host_str()
             .unwrap_or("<unknown>")
     );

src/commands/releases.rs

@@ -540,7 +540,7 @@ fn execute_set_commits<'a>(
     // make sure the release exists if projects are given
     if let Ok(projects) = ctx.get_projects(matches) {
         ctx.api.new_release(
-            &org,
+            org,
             &NewRelease {
                 version: version.into(),
                 projects,
@@ -570,7 +570,7 @@ fn execute_set_commits<'a>(
             }
             table.print();
         }
-        ctx.api.set_release_refs(&org, version, heads)?;
+        ctx.api.set_release_refs(org, version, heads)?;
     } else {
         let default_count = matches
             .value_of("initial-depth")
@@ -739,7 +739,7 @@ fn execute_info<'a>(ctx: &ReleaseContext<'_>, matches: &ArgMatches<'a>) -> Resul
     let version = matches.value_of("version").unwrap();
     let org = ctx.get_org()?;
     let project = ctx.get_project_default().ok();
-    let release = ctx.api.get_release(org, project.as_deref(), &version)?;
+    let release = ctx.api.get_release(org, project.as_deref(), version)?;
 
     // quiet mode just exists
     if matches.is_present("quiet") {
@@ -896,7 +896,7 @@ fn execute_files_upload<'a>(
         let ctx = &UploadContext {
             org,
             project: project.as_deref(),
-            release: &version,
+            release: version,
             dist,
             wait: matches.is_present("wait"),
         };
@@ -916,9 +916,9 @@ fn execute_files_upload<'a>(
         if let Some(artifact) = ctx.api.upload_release_file(
             org,
             project.as_deref(),
-            &version,
+            version,
             &FileContents::FromPath(path),
-            &name,
+            name,
             dist,
             Some(&headers[..]),
             ProgressBarMode::Request,
@@ -1096,7 +1096,7 @@ fn execute_files_upload_sourcemaps<'a>(
 
     // make sure the release exists
     let release = ctx.api.new_release(
-        &org,
+        org,
         &NewRelease {
             version: version.into(),
             projects: ctx.get_projects(matches)?,

src/commands/uninstall.rs

@@ -50,11 +50,11 @@ pub fn execute(matches: &ArgMatches<'_>) -> Result<(), Error> {
         return Err(QuietExit(1).into());
     }
 
-    if !matches.is_present("confirm") {
-        if !prompt_to_continue("Do you really want to uninstall sentry-cli?")? {
-            println!("Aborted!");
-            return Ok(());
-        }
+    if !matches.is_present("confirm")
+        && !prompt_to_continue("Do you really want to uninstall sentry-cli?")?
+    {
+        println!("Aborted!");
+        return Ok(());
     }
 
     if !is_writable(&exe) {

src/utils/dif_upload.rs

@@ -204,8 +204,8 @@ impl<'data> DifMatch<'data> {
     pub fn data(&self) -> &[u8] {
         match self.dif.get() {
             ParsedDif::Object(ref obj) => obj.data(),
-            ParsedDif::BcSymbolMap(_) => &self.dif.owner(),
-            ParsedDif::UuidMap(_) => &self.dif.owner(),
+            ParsedDif::BcSymbolMap(_) => self.dif.owner(),
+            ParsedDif::UuidMap(_) => self.dif.owner(),
         }
     }
 
@@ -896,7 +896,7 @@ fn resolve_hidden_symbols<'a>(dif: DifMatch<'a>, symbol_map: &Path) -> Result<Di
     // Copy the UUID plists
     for (name, view) in dif.attachments().unwrap() {
         let mut plist = File::create(temp_dir.path().join(name))?;
-        plist.write_all(&view)?;
+        plist.write_all(view)?;
         plist.sync_data()?;
     }
 
@@ -1498,7 +1498,7 @@ fn upload_in_batches(
             style(">").dim(),
             style(batch.len()).yellow()
         );
-        let archive = create_batch_archive(&batch)?;
+        let archive = create_batch_archive(batch)?;
 
         println!("{} Uploading debug symbol files", style(">").dim());
         dsyms.extend(api.upload_dif_archive(&options.org, &options.project, archive.path())?);
@@ -1937,7 +1937,7 @@ impl DifUpload {
         }
 
         // Skip if this DIF does not have features we want.
-        if !self.valid_features(&dif) {
+        if !self.valid_features(dif) {
             debug!("skipping {} because of features", dif.name);
             return false;
         }

src/utils/file_search.rs

@@ -129,7 +129,7 @@ impl ReleaseFileSearch {
         if !&self.ignores.is_empty() {
             let mut override_builder = OverrideBuilder::new(&self.path);
             for ignore in &self.ignores {
-                override_builder.add(&ignore)?;
+                override_builder.add(ignore)?;
             }
             builder.overrides(override_builder.build()?);
         }

src/utils/file_upload.rs

@@ -173,13 +173,8 @@ fn upload_files_parallel(
                 if let Some(old_id) =
                     release_files.get(&(context.dist.map(|x| x.into()), file.url.clone()))
                 {
-                    api.delete_release_file(
-                        context.org,
-                        context.project,
-                        &context.release,
-                        &old_id,
-                    )
-                    .ok();
+                    api.delete_release_file(context.org, context.project, context.release, old_id)
+                        .ok();
                 }
 
                 api.upload_release_file(

src/utils/sourcemaps.rs

@@ -145,7 +145,7 @@ fn guess_sourcemap_reference(sourcemaps: &HashSet<String>, min_url: &str) -> Res
         let mut parts: Vec<_> = ext.split('.').collect();
         if parts.len() > 1 {
             let parts_len = parts.len();
-            parts[parts_len - 1] = &map_ext;
+            parts[parts_len - 1] = map_ext;
             let new_ext = parts.join(".");
             if sourcemaps.contains(&unsplit_url(path, basename, Some(&new_ext))) {
                 return Ok(unsplit_url(None, basename, Some(&new_ext)));
@@ -389,12 +389,12 @@ impl SourceMapProcessor {
         self.sources.remove(&sourcemap_url);
 
         let ram_bundle_iter =
-            sourcemap::ram_bundle::split_ram_bundle(&ram_bundle, &sourcemap_index).unwrap();
+            sourcemap::ram_bundle::split_ram_bundle(ram_bundle, &sourcemap_index).unwrap();
         for result in ram_bundle_iter {
             let (name, sourceview, sourcemap) = result?;
 
             debug!("Inserting source for {}", name);
-            let source_url = join_url(&bundle_source_url, &name)?;
+            let source_url = join_url(bundle_source_url, &name)?;
             self.sources.insert(
                 source_url.clone(),
                 ReleaseFile {