Fix: handle empty bin strings (#6515)

* Fix: handle empty bin strings

When the package.json of a dependency sets `bin` to an empty string,
do not normalize it. This prevents creating a symlink to the
dependency's root directory within the `.bin` directory if bin-links
are enabled

* Add an entry to the changelog

* Add a test to make sure empty binlinks aren't created

Test was suggested/provided by @rally25rs

* Fix empty bin string test on Windows

Author: rhburrows

CHANGELOG.md

@@ -4,6 +4,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa
 
 ## Master
 
+- Fixes handling of empty string entries for bin in package.json
+
+  [#6515](https://github.com/yarnpkg/yarn/pull/6515) - [**Ryan Burrows**](https://github.com/rhburrows)
+
 - Adds support for basic auth for registries with paths, such as artifactory
 
   [#5322](https://github.com/yarnpkg/yarn/pull/5322) - [**Karolis Narkevicius**](https://twitter.com/KidkArolis)

__tests__/__snapshots__/normalize-manifest.js.snap

@@ -90,6 +90,12 @@ Array [
 ]
 `;
 
+exports[`empty bin string: empty bin string 1`] = `
+Array [
+  "foo: No license field",
+]
+`;
+
 exports[`engines array: engines array 1`] = `
 Array [
   "No license field",

__tests__/commands/install/bin-links.js

@@ -175,6 +175,16 @@ test('can use link protocol to install a package that would not be found via nod
   });
 });
 
+test('empty bin string does not create a link', (): Promise<void> => {
+  return runInstall({binLinks: true}, 'install-empty-bin', async config => {
+    const binScripts = await fs.walk(path.join(config.cwd, 'node_modules', '.bin'));
+    const linkCount = process.platform === 'win32' ? 2 : 1;
+    expect(binScripts).toHaveLength(linkCount);
+
+    expect(await linkAt(config, 'node_modules', '.bin', 'depB')).toEqual('../depB/depb.js');
+  });
+});
+
 describe('with nohoist', () => {
   // address https://github.com/yarnpkg/yarn/issues/5487
   test('nohoist bin should be linked to its own local module', (): Promise<void> => {

__tests__/fixtures/install/install-empty-bin/depA/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "depA",
+  "version": "0.0.0",
+  "bin": ""
+}

__tests__/fixtures/install/install-empty-bin/depB/depb.js

@@ -0,0 +1,2 @@
+#!/usr/bin/env node
+console.log('depB');

__tests__/fixtures/install/install-empty-bin/depB/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "depB",
+  "version": "0.0.0",
+  "bin": "./depb.js"
+}

__tests__/fixtures/install/install-empty-bin/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "test",
+  "version": "0.0.0",
+  "bin": "",
+  "dependencies": {
+    "depA": "file:depA",
+    "depB": "file:depB"
+  }
+}

__tests__/fixtures/normalize-manifest/empty bin string/actual.json

@@ -0,0 +1,4 @@
+{
+  "name": "foo",
+  "bin": ""
+}

__tests__/fixtures/normalize-manifest/empty bin string/expected.json

@@ -0,0 +1,5 @@
+{
+  "name": "foo",
+  "version": "",
+  "bin": ""
+}

src/util/normalize-manifest/fix.js

@@ -153,7 +153,7 @@ export default (async function(
   // if the `bin` field is as string then expand it to an object with a single property
   // based on the original `bin` field and `name field`
   // { name: "foo", bin: "cli.js" } -> { name: "foo", bin: { foo: "cli.js" } }
-  if (typeof info.name === 'string' && typeof info.bin === 'string') {
+  if (typeof info.name === 'string' && typeof info.bin === 'string' && info.bin.length > 0) {
     // Remove scoped package name for consistency with NPM's bin field fixing behaviour
     const name = info.name.replace(/^@[^\/]+\//, '');
     info.bin = {[name]: info.bin};