Raise warning on invalid color scheme in commands

src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp

@@ -68,6 +68,8 @@ namespace SettingsModelLocalTests
 
         TEST_METHOD(ValidateKeybindingsWarnings);
 
+        TEST_METHOD(ValidateColorSchemeInCommands);
+
         TEST_METHOD(ValidateExecuteCommandlineWarning);
 
         TEST_METHOD(ValidateLegacyGlobalsWarning);
@@ -1325,6 +1327,140 @@ namespace SettingsModelLocalTests
         VERIFY_ARE_EQUAL(L"Campbell", settings->_allProfiles.GetAt(2).ColorSchemeName());
     }
 
+    void DeserializationTests::ValidateColorSchemeInCommands()
+    {
+        Log::Comment(NoThrowString().Format(
+            L"Ensure that setting a command's color scheme to a non-existent scheme causes a warning."));
+
+        const std::string settings0String{ R"(
+        {
+            "profiles": [
+                {
+                    "name" : "profile0",
+                    "colorScheme": "schemeOne"
+                }
+            ],
+            "schemes": [
+                {
+                    "name": "schemeOne",
+                    "foreground": "#111111"
+                }
+            ],
+            "actions": [
+                {
+                    "command": { "action": "setColorScheme", "colorScheme": "schemeOne" }
+                },
+                {
+                    "command": { "action": "setColorScheme", "colorScheme": "invalidScheme" }
+                }
+            ]
+        })" };
+
+        const std::string settings1String{ R"(
+        {
+            "profiles": [
+                {
+                    "name" : "profile0",
+                    "colorScheme": "schemeOne"
+                }
+            ],
+            "schemes": [
+                {
+                    "name": "schemeOne",
+                    "foreground": "#111111"
+                }
+            ],
+            "actions": [
+                {
+                    "command": { "action": "setColorScheme", "colorScheme": "schemeOne" }
+                },
+                {
+                    "name": "parent",
+                    "commands": [                        
+                        { "command": { "action": "setColorScheme", "colorScheme": "invalidScheme" } }
+                    ]
+                }
+            ]
+        })" };
+
+        const std::string settings2String{ R"(
+        {
+            "profiles": [
+                {
+                    "name" : "profile0",
+                    "colorScheme": "schemeOne"
+                }
+            ],
+            "schemes": [
+                {
+                    "name": "schemeOne",
+                    "foreground": "#111111"
+                }
+            ],
+            "actions": [
+                {
+                    "command": { "action": "setColorScheme", "colorScheme": "schemeOne" }
+                },
+                {
+                    "name": "grandparent",
+                    "commands": [                        
+                        {
+                            "name": "parent",
+                            "commands": [
+                                { 
+                                    "command": { "action": "setColorScheme", "colorScheme": "invalidScheme" }
+                                }
+                            ]
+                        }
+                    ]
+                }
+            ]
+        })" };
+
+        {
+            // Case 1: setColorScheme command with invalid scheme
+            Log::Comment(NoThrowString().Format(
+                L"Testing a simple command with invalid scheme"));
+            VerifyParseSucceeded(settings0String);
+
+            auto settings = winrt::make_self<implementation::CascadiaSettings>();
+            settings->_ParseJsonString(settings0String, false);
+            settings->LayerJson(settings->_userSettings);
+            settings->_ValidateColorSchemesInCommands();
+
+            VERIFY_ARE_EQUAL(1u, settings->_warnings.Size());
+            VERIFY_ARE_EQUAL(SettingsLoadWarnings::InvalidColorSchemeInCmd, settings->_warnings.GetAt(0));
+        }
+        {
+            // Case 2: nested setColorScheme command with invalid scheme
+            Log::Comment(NoThrowString().Format(
+                L"Testing a nested command with invalid scheme"));
+            VerifyParseSucceeded(settings1String);
+
+            auto settings = winrt::make_self<implementation::CascadiaSettings>();
+            settings->_ParseJsonString(settings1String, false);
+            settings->LayerJson(settings->_userSettings);
+            settings->_ValidateColorSchemesInCommands();
+
+            VERIFY_ARE_EQUAL(1u, settings->_warnings.Size());
+            VERIFY_ARE_EQUAL(SettingsLoadWarnings::InvalidColorSchemeInCmd, settings->_warnings.GetAt(0));
+        }
+        {
+            // Case 3: nested-in-nested setColorScheme command with invalid scheme
+            Log::Comment(NoThrowString().Format(
+                L"Testing a nested-in-nested command with invalid scheme"));
+            VerifyParseSucceeded(settings2String);
+
+            auto settings = winrt::make_self<implementation::CascadiaSettings>();
+            settings->_ParseJsonString(settings2String, false);
+            settings->LayerJson(settings->_userSettings);
+            settings->_ValidateColorSchemesInCommands();
+
+            VERIFY_ARE_EQUAL(1u, settings->_warnings.Size());
+            VERIFY_ARE_EQUAL(SettingsLoadWarnings::InvalidColorSchemeInCmd, settings->_warnings.GetAt(0));
+        }
+    }
+
     void DeserializationTests::TestHelperFunctions()
     {
         const std::string settings0String{ R"(

src/cascadia/TerminalApp/AppLogic.cpp

@@ -41,7 +41,8 @@ static const std::array<std::wstring_view, static_cast<uint32_t>(SettingsLoadWar
     USES_RESOURCE(L"MissingRequiredParameter"),
     USES_RESOURCE(L"LegacyGlobalsProperty"),
     USES_RESOURCE(L"FailedToParseCommandJson"),
-    USES_RESOURCE(L"FailedToWriteToSettings")
+    USES_RESOURCE(L"FailedToWriteToSettings"),
+    USES_RESOURCE(L"InvalidColorSchemeInCmd")
 };
 static const std::array<std::wstring_view, static_cast<uint32_t>(SettingsLoadErrors::ERRORS_SIZE)> settingsLoadErrorsLabels {
     USES_RESOURCE(L"NoProfilesText"),

src/cascadia/TerminalApp/Resources/en-US/Resources.resw

@@ -245,6 +245,10 @@
     <value>Failed to expand a command with "iterateOn" set. This command will be ignored.</value>
     <comment>{Locked="\"iterateOn\""} </comment>
   </data>
+  <data name="InvalidColorSchemeInCmd" xml:space="preserve">
+    <value>Found a command with an invalid "colorScheme". This command will be ignored. Make sure that when setting a "colorScheme", the value matches the "name" of a color scheme in the "schemes" list.</value>
+    <comment>{Locked="\"colorScheme\"","\"name\"","\"schemes\""}</comment>
+  </data>
   <data name="CmdCommandArgDesc" xml:space="preserve">
     <value>An optional command, with arguments, to be spawned in the new tab or pane</value>
   </data>

src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp

@@ -282,6 +282,8 @@ void CascadiaSettings::_ValidateSettings()
     // This will also catch other keybinding warnings, like from GH#4239
     _ValidateKeybindings();
 
+    _ValidateColorSchemesInCommands();
+
     _ValidateNoGlobalsKey();
 }
 
@@ -697,6 +699,64 @@ void CascadiaSettings::_ValidateKeybindings()
     }
 }
 
+// Method Description:
+// - Ensures that every "setColorScheme" command has a valid "color scheme" set.
+// Arguments:
+// - <none>
+// Return Value:
+// - <none>
+// - Appends a SettingsLoadWarnings::InvalidColorSchemeInCmd to our list of warnings if
+//   we find any command with an invalid color scheme.
+void CascadiaSettings::_ValidateColorSchemesInCommands()
+{
+    bool foundInvalidScheme{ false };
+    for (const auto& nameAndCmd : _globals->Commands())
+    {
+        if (_HasInvalidColorScheme(nameAndCmd.Value()))
+        {
+            foundInvalidScheme = true;
+            break;
+        }
+    }
+
+    if (foundInvalidScheme)
+    {
+        _warnings.Append(SettingsLoadWarnings::InvalidColorSchemeInCmd);
+    }
+}
+
+bool CascadiaSettings::_HasInvalidColorScheme(const Model::Command& command)
+{
+    bool invalid{ false };
+    if (command.HasNestedCommands())
+    {
+        for (const auto& nested : command.NestedCommands())
+        {
+            if (_HasInvalidColorScheme(nested.Value()))
+            {
+                invalid = true;
+                break;
+            }
+        }
+    }
+    else if (const auto& actionAndArgs = command.Action())
+    {
+        if (const auto& realArgs = actionAndArgs.Args().try_as<Model::SetColorSchemeArgs>())
+        {
+            auto cmdImpl{ winrt::get_self<Command>(command) };
+            // no need to validate iterable commands on color schemes
+            // they will be expanded to commands with a valid scheme name
+            if (cmdImpl->IterateOn() != ExpandCommandType::ColorSchemes &&
+                !_globals->ColorSchemes().HasKey(realArgs.SchemeName()))
+            {
+                invalid = true;
+            }
+        }
+    }
+
+    return invalid;
+}
+
 // Method Description:
 // - Checks for the presence of the legacy "globals" key in the user's
 //   settings.json. If this key is present, then they've probably got a pre-0.11

src/cascadia/TerminalSettingsModel/CascadiaSettings.h

@@ -145,8 +145,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         void _ValidateAllSchemesExist();
         void _ValidateMediaResources();
         void _ValidateKeybindings();
+        void _ValidateColorSchemesInCommands();
         void _ValidateNoGlobalsKey();
 
+        bool _HasInvalidColorScheme(const Model::Command& command);
+
         friend class SettingsModelLocalTests::SerializationTests;
         friend class SettingsModelLocalTests::DeserializationTests;
         friend class SettingsModelLocalTests::ProfileTests;

src/cascadia/TerminalSettingsModel/TerminalWarnings.idl

@@ -18,6 +18,7 @@ namespace Microsoft.Terminal.Settings.Model
         LegacyGlobalsProperty = 8,
         FailedToParseCommandJson = 9,
         FailedToWriteToSettings = 10,
+        InvalidColorSchemeInCmd = 11,
         WARNINGS_SIZE // IMPORTANT: This MUST be the last value in this enum. It's an unused placeholder.
     };