From 673e3c2df3e21e05ee40d3bb8da4a6fc095d92d1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Llu=C3=ADs=20Mart=C3=ADnez?= <lluis@tastaollet.es>
Date: Wed, 15 Mar 2023 10:56:46 +0100
Subject: [PATCH] Fix invalid JSON and YAML outputs for `outdated` command
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes issue #2104

The code for `internal/cli/core` and `internal/cli/lib` has been
refactored so that we can use only the `Get` functions from
`internal/cli/outdated` package and compose there composited object.

For regular text output, the new table will have some extra fields that
either for platforms or for libraries will be blank.

For JSON and YAML output, the resulting object will have the top-level
keys `platforms` and `libraries` which contain, respectively, the array
of outaded platforms and outdated libraries.

Signed-off-by: Lluís Martínez <lluis@tastaollet.es>
---
 docs/UPGRADING.md                             |  42 ++++++++
 internal/cli/core/list.go                     |  11 +-
 internal/cli/lib/list.go                      |  23 ++--
 internal/cli/outdated/outdated.go             | 100 +++++++++++++++++-
 .../integrationtest/outdated/outdated_test.go |   2 +-
 .../integrationtest/update/update_test.go     |   2 +-
 .../integrationtest/upgrade/upgrade_test.go   |   4 +-
 7 files changed, 168 insertions(+), 16 deletions(-)

diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md
index 819882bbb68..39c3bd8abc5 100644
--- a/docs/UPGRADING.md
+++ b/docs/UPGRADING.md
@@ -7,6 +7,48 @@ Here you can find a list of migration guides to handle breaking changes between
 Configuration file lookup in current working directory and its parents is dropped. The command line flag `--config-file`
 must be specified to use an alternative configuration file from the one in the data directory.
 
+### Command `outdated` output change
+
+For `text` format (default), the command prints now a single table for platforms and libraries instead of two separate
+tables.
+
+Similarly, for JSON and YAML formats, the command prints now a single valid object, with `platform` and `libraries`
+top-level keys. For example, for JSON output:
+
+```
+$ arduino-cli outdated --format json
+{
+  "platforms": [
+    {
+      "id": "arduino:avr",
+      "installed": "1.6.3",
+      "latest": "1.8.6",
+      "name": "Arduino AVR Boards",
+      ...
+    }
+  ],
+  "libraries": [
+    {
+      "library": {
+        "name": "USBHost",
+        "author": "Arduino",
+        "maintainer": "Arduino \u003cinfo@arduino.cc\u003e",
+        "category": "Device Control",
+        "version": "1.0.0",
+        ...
+      },
+      "release": {
+        "author": "Arduino",
+        "version": "1.0.5",
+        "maintainer": "Arduino \u003cinfo@arduino.cc\u003e",
+        "category": "Device Control",
+        ...
+      }
+    }
+  ]
+}
+```
+
 ## 0.31.0
 
 ### Added `post_install` script support for tools
diff --git a/internal/cli/core/list.go b/internal/cli/core/list.go
index b53a75e25b1..316b4275a73 100644
--- a/internal/cli/core/list.go
+++ b/internal/cli/core/list.go
@@ -52,8 +52,14 @@ func runListCommand(args []string, all bool, updatableOnly bool) {
 	List(inst, all, updatableOnly)
 }
 
-// List print a list of installed platforms.
+// List gets and prints a list of installed platforms.
 func List(inst *rpc.Instance, all bool, updatableOnly bool) {
+	platforms := GetList(inst, all, updatableOnly)
+	feedback.PrintResult(installedResult{platforms})
+}
+
+// GetList returns a list of installed platforms.
+func GetList(inst *rpc.Instance, all bool, updatableOnly bool) []*rpc.Platform {
 	platforms, err := core.GetPlatforms(&rpc.PlatformListRequest{
 		Instance:      inst,
 		UpdatableOnly: updatableOnly,
@@ -62,8 +68,7 @@ func List(inst *rpc.Instance, all bool, updatableOnly bool) {
 	if err != nil {
 		feedback.Fatal(tr("Error listing platforms: %v", err), feedback.ErrGeneric)
 	}
-
-	feedback.PrintResult(installedResult{platforms})
+	return platforms
 }
 
 // output from this command requires special formatting, let's create a dedicated
diff --git a/internal/cli/lib/list.go b/internal/cli/lib/list.go
index 3e65cddcd65..57674c38a23 100644
--- a/internal/cli/lib/list.go
+++ b/internal/cli/lib/list.go
@@ -60,8 +60,23 @@ func runListCommand(args []string, all bool, updatable bool) {
 	List(instance, args, all, updatable)
 }
 
-// List lists all the installed libraries.
+// List gets and prints a list of installed libraries.
 func List(instance *rpc.Instance, args []string, all bool, updatable bool) {
+	installedLibs := GetList(instance, args, all, updatable)
+	feedback.PrintResult(installedResult{
+		onlyUpdates:   updatable,
+		installedLibs: installedLibs,
+	})
+	logrus.Info("Done")
+}
+
+// GetList returns a list of installed libraries.
+func GetList(
+	instance *rpc.Instance,
+	args []string,
+	all bool,
+	updatable bool,
+) []*rpc.InstalledLibrary {
 	name := ""
 	if len(args) > 0 {
 		name = args[0]
@@ -95,11 +110,7 @@ func List(instance *rpc.Instance, args []string, all bool, updatable bool) {
 		libs = []*rpc.InstalledLibrary{}
 	}
 
-	feedback.PrintResult(installedResult{
-		onlyUpdates:   updatable,
-		installedLibs: libs,
-	})
-	logrus.Info("Done")
+	return libs
 }
 
 // output from this command requires special formatting, let's create a dedicated
diff --git a/internal/cli/outdated/outdated.go b/internal/cli/outdated/outdated.go
index 35dfab911da..8a56f21b36a 100644
--- a/internal/cli/outdated/outdated.go
+++ b/internal/cli/outdated/outdated.go
@@ -16,13 +16,18 @@
 package outdated
 
 import (
+	"fmt"
 	"os"
+	"sort"
+	"strings"
 
 	"github.com/arduino/arduino-cli/i18n"
 	"github.com/arduino/arduino-cli/internal/cli/core"
+	"github.com/arduino/arduino-cli/internal/cli/feedback"
 	"github.com/arduino/arduino-cli/internal/cli/instance"
 	"github.com/arduino/arduino-cli/internal/cli/lib"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
+	"github.com/arduino/arduino-cli/table"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
@@ -49,8 +54,97 @@ func runOutdatedCommand(cmd *cobra.Command, args []string) {
 	Outdated(inst)
 }
 
-// Outdated returns a list of outdated platforms and libraries
+// Outdated prints a list of outdated platforms and libraries
 func Outdated(inst *rpc.Instance) {
-	core.List(inst, false, true)
-	lib.List(inst, []string{}, false, true)
+	feedback.PrintResult(
+		outdatedResult{core.GetList(inst, false, true), lib.GetList(inst, []string{}, false, true)},
+	)
+}
+
+// output from this command requires special formatting, let's create a dedicated
+// feedback.Result implementation
+type outdatedResult struct {
+	Platforms     []*rpc.Platform         `json:"platforms,omitempty"`
+	InstalledLibs []*rpc.InstalledLibrary `json:"libraries,omitempty"`
+}
+
+func (ir outdatedResult) Data() interface{} {
+	return &ir
+}
+
+func (ir outdatedResult) String() string {
+	if len(ir.Platforms) == 0 && len(ir.InstalledLibs) == 0 {
+		return tr("No outdated platforms or libraries found.")
+	}
+
+	// A table useful both for platforms and libraries, where some of the fields will be blank.
+	t := table.New()
+	t.SetHeader(
+		tr("ID"),
+		tr("Name"),
+		tr("Installed"),
+		tr("Latest"),
+		tr("Location"),
+		tr("Description"),
+	)
+	t.SetColumnWidthMode(2, table.Average)
+	t.SetColumnWidthMode(3, table.Average)
+	t.SetColumnWidthMode(5, table.Average)
+
+	// Based on internal/cli/core/list.go
+	for _, p := range ir.Platforms {
+		name := p.Name
+		if p.Deprecated {
+			name = fmt.Sprintf("[%s] %s", tr("DEPRECATED"), name)
+		}
+		t.AddRow(p.Id, name, p.Installed, p.Latest, "", "")
+	}
+
+	// Based on internal/cli/lib/list.go
+	sort.Slice(ir.InstalledLibs, func(i, j int) bool {
+		return strings.ToLower(
+			ir.InstalledLibs[i].Library.Name,
+		) < strings.ToLower(
+			ir.InstalledLibs[j].Library.Name,
+		) ||
+			strings.ToLower(
+				ir.InstalledLibs[i].Library.ContainerPlatform,
+			) < strings.ToLower(
+				ir.InstalledLibs[j].Library.ContainerPlatform,
+			)
+	})
+	lastName := ""
+	for _, libMeta := range ir.InstalledLibs {
+		lib := libMeta.GetLibrary()
+		name := lib.Name
+		if name == lastName {
+			name = ` "`
+		} else {
+			lastName = name
+		}
+
+		location := lib.GetLocation().String()
+		if lib.ContainerPlatform != "" {
+			location = lib.GetContainerPlatform()
+		}
+
+		available := ""
+		sentence := ""
+		if libMeta.GetRelease() != nil {
+			available = libMeta.GetRelease().GetVersion()
+			sentence = lib.Sentence
+		}
+
+		if available == "" {
+			available = "-"
+		}
+		if sentence == "" {
+			sentence = "-"
+		} else if len(sentence) > 40 {
+			sentence = sentence[:37] + "..."
+		}
+		t.AddRow("", name, lib.Version, available, location, sentence)
+	}
+
+	return t.Render()
 }
diff --git a/internal/integrationtest/outdated/outdated_test.go b/internal/integrationtest/outdated/outdated_test.go
index 64e1f39304e..77f7c54bcd4 100644
--- a/internal/integrationtest/outdated/outdated_test.go
+++ b/internal/integrationtest/outdated/outdated_test.go
@@ -53,7 +53,7 @@ func TestOutdated(t *testing.T) {
 		lines[i] = strings.TrimSpace(lines[i])
 	}
 	require.Contains(t, lines[1], "Arduino AVR Boards")
-	require.Contains(t, lines[4], "USBHost")
+	require.Contains(t, lines[2], "USBHost")
 }
 
 func TestOutdatedUsingLibraryWithInvalidVersion(t *testing.T) {
diff --git a/internal/integrationtest/update/update_test.go b/internal/integrationtest/update/update_test.go
index 8bfe7a46c49..927ae0e3c02 100644
--- a/internal/integrationtest/update/update_test.go
+++ b/internal/integrationtest/update/update_test.go
@@ -67,7 +67,7 @@ func TestUpdateShowingOutdated(t *testing.T) {
 	require.Contains(t, lines[0], "Downloading index: library_index.tar.bz2 downloaded")
 	require.Contains(t, lines[1], "Downloading index: package_index.tar.bz2 downloaded")
 	require.Contains(t, lines[3], "Arduino AVR Boards")
-	require.Contains(t, lines[6], "USBHost")
+	require.Contains(t, lines[4], "USBHost")
 }
 
 func TestUpdateWithUrlNotFound(t *testing.T) {
diff --git a/internal/integrationtest/upgrade/upgrade_test.go b/internal/integrationtest/upgrade/upgrade_test.go
index ffd8ecbe8e7..0b041a9bcc2 100644
--- a/internal/integrationtest/upgrade/upgrade_test.go
+++ b/internal/integrationtest/upgrade/upgrade_test.go
@@ -50,7 +50,7 @@ func TestUpgrade(t *testing.T) {
 	require.NoError(t, err)
 	lines := strings.Split(string(stdout), "\n")
 	require.Contains(t, lines[1], "Arduino AVR Boards")
-	require.Contains(t, lines[4], "USBHost")
+	require.Contains(t, lines[2], "USBHost")
 
 	_, _, err = cli.Run("upgrade")
 	require.NoError(t, err)
@@ -58,7 +58,7 @@ func TestUpgrade(t *testing.T) {
 	// Verifies cores and libraries have been updated
 	stdout, _, err = cli.Run("outdated")
 	require.NoError(t, err)
-	require.Contains(t, string(stdout), "No libraries update is available.")
+	require.Contains(t, string(stdout), "No outdated platforms or libraries found.")
 }
 
 func TestUpgradeUsingLibraryWithInvalidVersion(t *testing.T) {