Shell: Convert all immediately convertible fallible functions to ErrorOr

Author: https://github.com/alimpfard
Commit: 0c28fd41ed
Pull-request: https://github.com/SerenityOS/serenity/pull/17538
Reviewed-by: https://github.com/ADKaster

Userland/Shell/AST.cpp

@@ -1000,7 +1000,7 @@ ErrorOr<RefPtr<Value>> DoubleQuotedString::run(RefPtr<Shell> shell)
 
     builder.join(""sv, values);
 
-    return make_ref_counted<StringValue>(builder.to_string().release_value_but_fixme_should_propagate_errors());
+    return make_ref_counted<StringValue>(TRY(builder.to_string()));
 }
 
 ErrorOr<void> DoubleQuotedString::highlight_in_editor(Line::Editor& editor, Shell& shell, HighlightMetadata metadata)
@@ -1711,7 +1711,7 @@ ErrorOr<void> Execute::for_each_entry(RefPtr<Shell> shell, Function<ErrorOr<Iter
     if (!shell)
         return {};
 
-    auto commands = shell->expand_aliases(move(unexpanded_commands));
+    auto commands = TRY(shell->expand_aliases(move(unexpanded_commands)));
 
     if (m_capture_stdout) {
         // Make sure that we're going to be running _something_.
@@ -1751,12 +1751,12 @@ ErrorOr<void> Execute::for_each_entry(RefPtr<Shell> shell, Function<ErrorOr<Iter
             NothingLeft,
         };
         auto check_and_call = [&]() -> ErrorOr<CheckResult> {
-            auto ifs = shell->local_variable_or("IFS"sv, "\n"sv);
+            auto ifs = TRY(shell->local_variable_or("IFS"sv, "\n"sv));
 
-            if (auto offset = stream.offset_of(ifs.bytes()).release_value_but_fixme_should_propagate_errors(); offset.has_value()) {
+            if (auto offset = TRY(stream.offset_of(ifs.bytes())); offset.has_value()) {
                 auto line_end = offset.value();
                 if (line_end == 0) {
-                    stream.discard(ifs.length()).release_value_but_fixme_should_propagate_errors();
+                    TRY(stream.discard(ifs.length()));
 
                     if (shell->options.inline_exec_keep_empty_segments)
                         if (TRY(callback(make_ref_counted<StringValue>(String {}))) == IterationDecision::Break) {
@@ -3741,7 +3741,7 @@ StringValue::~StringValue()
 ErrorOr<String> StringValue::resolve_as_string(RefPtr<Shell> shell)
 {
     if (m_split.is_empty())
-        return m_string;
+        return TRY(resolve_slices(shell, String { m_string }, m_slices));
     return Value::resolve_as_string(shell);
 }
 
@@ -3799,7 +3799,7 @@ ErrorOr<String> SimpleVariableValue::resolve_as_string(RefPtr<Shell> shell)
         return resolve_slices(shell, String {}, m_slices);
 
     if (auto value = TRY(resolve_without_cast(shell)); value != this)
-        return value->resolve_as_string(shell);
+        return resolve_slices(shell, TRY(value->resolve_as_string(shell)), m_slices);
 
     auto name = m_name.to_deprecated_string();
     char* env_value = getenv(name.characters());
@@ -3826,7 +3826,7 @@ ErrorOr<NonnullRefPtr<Value>> SimpleVariableValue::resolve_without_cast(RefPtr<S
 {
     VERIFY(shell);
 
-    if (auto value = shell->lookup_local_variable(m_name)) {
+    if (auto value = TRY(shell->lookup_local_variable(m_name))) {
         auto result = value.release_nonnull();
         // If a slice is applied, add it.
         if (!m_slices.is_empty())
@@ -3868,11 +3868,11 @@ ErrorOr<Vector<String>> SpecialVariableValue::resolve_as_list(RefPtr<Shell> shel
     case '$':
         return { resolve_slices(shell, Vector { TRY(String::number(getpid())) }, m_slices) };
     case '*':
-        if (auto argv = shell->lookup_local_variable("ARGV"sv))
+        if (auto argv = TRY(shell->lookup_local_variable("ARGV"sv)))
             return resolve_slices(shell, TRY(const_cast<Value&>(*argv).resolve_as_list(shell)), m_slices);
         return resolve_slices(shell, Vector<String> {}, m_slices);
     case '#':
-        if (auto argv = shell->lookup_local_variable("ARGV"sv)) {
+        if (auto argv = TRY(shell->lookup_local_variable("ARGV"sv))) {
             if (argv->is_list()) {
                 auto list_argv = static_cast<AST::ListValue const*>(argv.ptr());
                 return { resolve_slices(shell, Vector { TRY(String::number(list_argv->values().size())) }, m_slices) };

Userland/Shell/Builtin.cpp

@@ -544,7 +544,7 @@ ErrorOr<int> Shell::builtin_export(Main::Arguments arguments)
         auto parts = value.split_limit('=', 2);
 
         if (parts.size() == 1) {
-            auto value = lookup_local_variable(parts[0]);
+            auto value = TRY(lookup_local_variable(parts[0]));
             if (value) {
                 auto values = TRY(const_cast<AST::Value&>(*value).resolve_as_list(*this));
                 StringBuilder builder;
@@ -932,7 +932,7 @@ ErrorOr<int> Shell::builtin_shift(Main::Arguments arguments)
     if (count < 1)
         return 0;
 
-    auto argv_ = lookup_local_variable("ARGV"sv);
+    auto argv_ = TRY(lookup_local_variable("ARGV"sv));
     if (!argv_) {
         warnln("shift: ARGV is unset");
         return 1;
@@ -965,7 +965,7 @@ ErrorOr<int> Shell::builtin_source(Main::Arguments arguments)
     if (!parser.parse(arguments))
         return 1;
 
-    auto previous_argv = lookup_local_variable("ARGV"sv);
+    auto previous_argv = TRY(lookup_local_variable("ARGV"sv));
     ScopeGuard guard { [&] {
         if (!args.is_empty())
             set_local_variable("ARGV", const_cast<AST::Value&>(*previous_argv));
@@ -1008,7 +1008,7 @@ ErrorOr<int> Shell::builtin_time(Main::Arguments arguments)
     for (auto& arg : args)
         command.argv.append(TRY(String::from_utf8(arg)));
 
-    auto commands = expand_aliases({ move(command) });
+    auto commands = TRY(expand_aliases({ move(command) }));
 
     AK::Statistics iteration_times;
 
@@ -1146,7 +1146,7 @@ ErrorOr<int> Shell::builtin_unset(Main::Arguments arguments)
         if (!did_touch_path && value == "PATH"sv)
             did_touch_path = true;
 
-        if (lookup_local_variable(value)) {
+        if (TRY(lookup_local_variable(value)) != nullptr) {
             unset_local_variable(value);
         } else {
             unsetenv(value.characters());
@@ -1175,7 +1175,7 @@ ErrorOr<int> Shell::builtin_not(Main::Arguments arguments)
     for (auto& arg : args)
         command.argv.unchecked_append(TRY(String::from_utf8(arg)));
 
-    auto commands = expand_aliases({ move(command) });
+    auto commands = TRY(expand_aliases({ move(command) }));
     int exit_code = 1;
     auto found_a_job = false;
     for (auto& job : run_commands(commands)) {
@@ -1351,7 +1351,7 @@ ErrorOr<int> Shell::builtin_argsparser_parse(Main::Arguments arguments)
     };
 
     auto enlist = [&](auto name, auto value) -> ErrorOr<NonnullRefPtr<AST::Value>> {
-        auto variable = lookup_local_variable(name);
+        auto variable = TRY(lookup_local_variable(name));
         if (variable) {
             auto list = TRY(const_cast<AST::Value&>(*variable).resolve_as_list(*this));
             auto new_value = TRY(value->resolve_as_string(*this));

Userland/Shell/ImmediateFunctions.cpp

@@ -465,7 +465,7 @@ ErrorOr<RefPtr<AST::Node>> Shell::immediate_value_or_default(AST::ImmediateExpre
     }
 
     auto name = TRY(TRY(const_cast<AST::Node&>(arguments.first()).run(*this))->resolve_as_string(*this));
-    if (!local_variable_or(name, ""sv).is_empty())
+    if (!TRY(local_variable_or(name, ""sv)).is_empty())
         return make_ref_counted<AST::SimpleVariable>(invoking_node.position(), name);
 
     return arguments.last();
@@ -479,7 +479,7 @@ ErrorOr<RefPtr<AST::Node>> Shell::immediate_assign_default(AST::ImmediateExpress
     }
 
     auto name = TRY(TRY(const_cast<AST::Node&>(arguments.first()).run(*this))->resolve_as_string(*this));
-    if (!local_variable_or(name, ""sv).is_empty())
+    if (!TRY(local_variable_or(name, ""sv)).is_empty())
         return make_ref_counted<AST::SimpleVariable>(invoking_node.position(), name);
 
     auto value = TRY(TRY(const_cast<AST::Node&>(arguments.last()).run(*this))->resolve_without_cast(*this));
@@ -496,7 +496,7 @@ ErrorOr<RefPtr<AST::Node>> Shell::immediate_error_if_empty(AST::ImmediateExpress
     }
 
     auto name = TRY(TRY(const_cast<AST::Node&>(arguments.first()).run(*this))->resolve_as_string(*this));
-    if (!local_variable_or(name, ""sv).is_empty())
+    if (!TRY(local_variable_or(name, ""sv)).is_empty())
         return make_ref_counted<AST::SimpleVariable>(invoking_node.position(), name);
 
     auto error_value = TRY(TRY(const_cast<AST::Node&>(arguments.last()).run(*this))->resolve_as_string(*this));

Userland/Shell/Parser.cpp

@@ -1903,8 +1903,8 @@ RefPtr<AST::Node> Parser::parse_bareword()
 
         auto first_slash_index = string.find_byte_offset('/');
         if (first_slash_index.has_value()) {
-            username = string.substring_from_byte_offset(1, *first_slash_index).release_value_but_fixme_should_propagate_errors();
+            username = string.substring_from_byte_offset(1, *first_slash_index - 1).release_value_but_fixme_should_propagate_errors();
-            string = string.substring_from_byte_offset(*first_slash_index + 1).release_value_but_fixme_should_propagate_errors();
+            string = string.substring_from_byte_offset(*first_slash_index).release_value_but_fixme_should_propagate_errors();
         } else {
             username = string.substring_from_byte_offset(1).release_value_but_fixme_should_propagate_errors();
             string = {};

Userland/Shell/Shell.cpp

@@ -289,11 +289,11 @@ Vector<DeprecatedString> Shell::expand_globs(Vector<StringView> path_segments, S
     }
 }
 
-Vector<AST::Command> Shell::expand_aliases(Vector<AST::Command> initial_commands)
+ErrorOr<Vector<AST::Command>> Shell::expand_aliases(Vector<AST::Command> initial_commands)
 {
     Vector<AST::Command> commands;
 
-    Function<void(AST::Command&)> resolve_aliases_and_append = [&](auto& command) {
+    Function<ErrorOr<void>(AST::Command&)> resolve_aliases_and_append = [&](auto& command) -> ErrorOr<void> {
         if (!command.argv.is_empty()) {
             auto alias = resolve_alias(command.argv[0]);
             if (!alias.is_null()) {
@@ -308,12 +308,12 @@ Vector<AST::Command> Shell::expand_aliases(Vector<AST::Command> initial_commands
                     NonnullRefPtr<AST::Node> substitute = adopt_ref(*new AST::Join(subcommand_nonnull->position(),
                         subcommand_nonnull,
                         adopt_ref(*new AST::CommandLiteral(subcommand_nonnull->position(), command))));
-                    auto res = substitute->run(*this).release_value_but_fixme_should_propagate_errors();
+                    auto res = TRY(substitute->run(*this));
-                    for (auto& subst_command : res->resolve_as_commands(*this).release_value_but_fixme_should_propagate_errors()) {
+                    for (auto& subst_command : TRY(res->resolve_as_commands(*this))) {
                         if (!subst_command.argv.is_empty() && subst_command.argv.first() == argv0) // Disallow an alias resolving to itself.
                             commands.append(subst_command);
                         else
-                            resolve_aliases_and_append(subst_command);
+                            TRY(resolve_aliases_and_append(subst_command));
                     }
                 } else {
                     commands.append(command);
@@ -324,10 +324,12 @@ Vector<AST::Command> Shell::expand_aliases(Vector<AST::Command> initial_commands
         } else {
             commands.append(command);
         }
+
+        return {};
     };
 
     for (auto& command : initial_commands)
-        resolve_aliases_and_append(command);
+        TRY(resolve_aliases_and_append(command));
 
     return commands;
 }
@@ -350,7 +352,7 @@ Shell::LocalFrame* Shell::find_frame_containing_local_variable(StringView name)
     return nullptr;
 }
 
-RefPtr<AST::Value const> Shell::lookup_local_variable(StringView name) const
+ErrorOr<RefPtr<AST::Value const>> Shell::lookup_local_variable(StringView name) const
 {
     if (auto* frame = find_frame_containing_local_variable(name))
         return frame->local_variables.get(name).value();
@@ -361,13 +363,13 @@ RefPtr<AST::Value const> Shell::lookup_local_variable(StringView name) const
     return nullptr;
 }
 
-RefPtr<AST::Value const> Shell::get_argument(size_t index) const
+ErrorOr<RefPtr<AST::Value const>> Shell::get_argument(size_t index) const
 {
     if (index == 0)
-        return adopt_ref(*new AST::StringValue(String::from_deprecated_string(current_script).release_value_but_fixme_should_propagate_errors()));
+        return adopt_ref(*new AST::StringValue(TRY(String::from_deprecated_string(current_script))));
 
     --index;
-    if (auto argv = lookup_local_variable("ARGV"sv)) {
+    if (auto argv = TRY(lookup_local_variable("ARGV"sv))) {
         if (argv->is_list_without_resolution()) {
             AST::ListValue const* list = static_cast<AST::ListValue const*>(argv.ptr());
             if (list->values().size() <= index)
@@ -385,12 +387,12 @@ RefPtr<AST::Value const> Shell::get_argument(size_t index) const
     return nullptr;
 }
 
-DeprecatedString Shell::local_variable_or(StringView name, DeprecatedString const& replacement) const
+ErrorOr<DeprecatedString> Shell::local_variable_or(StringView name, DeprecatedString const& replacement) const
 {
-    auto value = lookup_local_variable(name);
+    auto value = TRY(lookup_local_variable(name));
     if (value) {
         StringBuilder builder;
-        builder.join(' ', const_cast<AST::Value&>(*value).resolve_as_list(const_cast<Shell&>(*this)).release_value_but_fixme_should_propagate_errors());
+        builder.join(' ', TRY(const_cast<AST::Value&>(*value).resolve_as_list(const_cast<Shell&>(*this))));
         return builder.to_deprecated_string();
     }
     return replacement;
@@ -1085,10 +1087,17 @@ bool Shell::is_allowed_to_modify_termios(const AST::Command& command) const
         return false;
 
     auto value = lookup_local_variable("PROGRAMS_ALLOWED_TO_MODIFY_DEFAULT_TERMIOS"sv);
-    if (!value)
+    if (value.is_error())
         return false;
 
-    return const_cast<AST::Value&>(*value).resolve_as_list(const_cast<Shell&>(*this)).release_value_but_fixme_should_propagate_errors().contains_slow(command.argv[0]);
+    if (!value.value())
+        return false;
+
+    auto result = const_cast<AST::Value&>(*value.value()).resolve_as_list(const_cast<Shell&>(*this));
+    if (result.is_error())
+        return false;
+
+    return result.value().contains_slow(command.argv[0]);
 }
 
 void Shell::restore_ios()
@@ -1428,13 +1437,13 @@ void Shell::remove_entry_from_cache(StringView entry)
         cached_path.remove(index);
 }
 
-void Shell::highlight(Line::Editor& editor) const
+ErrorOr<void> Shell::highlight(Line::Editor& editor) const
 {
     auto line = editor.line();
     auto ast = parse(line, m_is_interactive);
     if (!ast)
-        return;
+        return {};
-    ast->highlight_in_editor(editor, const_cast<Shell&>(*this)).release_value_but_fixme_should_propagate_errors();
+    return ast->highlight_in_editor(editor, const_cast<Shell&>(*this));
 }
 
 Vector<Line::CompletionSuggestion> Shell::complete()
@@ -1697,15 +1706,15 @@ ErrorOr<Vector<Line::CompletionSuggestion>> Shell::complete_via_program_itself(s
 
     AST::Command completion_command;
     completion_command.argv.append(program_name_storage);
-    completion_command = expand_aliases({ completion_command }).last();
+    completion_command = TRY(expand_aliases({ completion_command })).last();
 
-    auto completion_utility_name = String::formatted("_complete_{}", completion_command.argv[0]).release_value_but_fixme_should_propagate_errors();
+    auto completion_utility_name = TRY(String::formatted("_complete_{}", completion_command.argv[0]));
     if (binary_search(cached_path.span(), completion_utility_name, nullptr, RunnablePathComparator {}) != nullptr)
         completion_command.argv[0] = completion_utility_name;
     else if (!options.invoke_program_for_autocomplete)
         return Error::from_string_literal("Refusing to use the program itself as completion source");
 
-    completion_command.argv.extend({ String::from_utf8("--complete"sv).release_value_but_fixme_should_propagate_errors(), String::from_utf8_short_string("--"sv) });
+    completion_command.argv.extend({ TRY(String::from_utf8("--complete"sv)), String::from_utf8_short_string("--"sv) });
 
     struct Visitor : public AST::NodeVisitor {
         Visitor(Shell& shell, AST::Position position)
@@ -1874,7 +1883,7 @@ ErrorOr<Vector<Line::CompletionSuggestion>> Shell::complete_via_program_itself(s
 
     completion_command.argv.extend(visitor.list());
 
-    auto devnull = String::from_utf8("/dev/null"sv).release_value_but_fixme_should_propagate_errors();
+    auto devnull = TRY(String::from_utf8("/dev/null"sv));
     completion_command.should_wait = true;
     completion_command.redirections.append(AST::PathRedirection::create(devnull, STDERR_FILENO, AST::PathRedirection::Write));
     completion_command.redirections.append(AST::PathRedirection::create(devnull, STDIN_FILENO, AST::PathRedirection::Read));

Userland/Shell/Shell.h

@@ -165,15 +165,15 @@ public:
     static DeprecatedString expand_tilde(StringView expression);
     static Vector<DeprecatedString> expand_globs(StringView path, StringView base);
     static Vector<DeprecatedString> expand_globs(Vector<StringView> path_segments, StringView base);
-    Vector<AST::Command> expand_aliases(Vector<AST::Command>);
+    ErrorOr<Vector<AST::Command>> expand_aliases(Vector<AST::Command>);
     DeprecatedString resolve_path(DeprecatedString) const;
     DeprecatedString resolve_alias(StringView) const;
 
     static bool has_history_event(StringView);
 
-    RefPtr<AST::Value const> get_argument(size_t) const;
+    ErrorOr<RefPtr<AST::Value const>> get_argument(size_t) const;
-    RefPtr<AST::Value const> lookup_local_variable(StringView) const;
+    ErrorOr<RefPtr<AST::Value const>> lookup_local_variable(StringView) const;
-    DeprecatedString local_variable_or(StringView, DeprecatedString const&) const;
+    ErrorOr<DeprecatedString> local_variable_or(StringView, DeprecatedString const&) const;
     void set_local_variable(DeprecatedString const&, RefPtr<AST::Value>, bool only_in_current_frame = false);
     void unset_local_variable(StringView, bool only_in_current_frame = false);
 
@@ -276,7 +276,7 @@ public:
         No
     };
 
-    void highlight(Line::Editor&) const;
+    ErrorOr<void> highlight(Line::Editor&) const;
     Vector<Line::CompletionSuggestion> complete();
     Vector<Line::CompletionSuggestion> complete(StringView);
     Vector<Line::CompletionSuggestion> complete_program_name(StringView, size_t offset, EscapeMode = EscapeMode::Bareword);

Userland/Shell/main.cpp

@@ -83,7 +83,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
                 if (cursor >= 0)
                     editor.set_cursor(cursor);
             }
-            shell->highlight(editor);
+            (void)shell->highlight(editor);
         };
         editor->on_tab_complete = [&](const Line::Editor&) {
             return shell->complete();