eliminate "useless parentheses" lint errors

Some `oclint` errors regarding "useless parentheses" are meaningfull. But
the vast majority are bogus in as much as removing the parentheses reduces
readability. So fix a few of the egregious uses and otherwise suppress
that error.

.oclint

@@ -2,7 +2,18 @@ rules:
 rule-configurations:
     # This is the default value (as of the time I wrote this) but I'm making
     # it explicit since it needs to agree with the value used by clang-format.
-    # Thus, if we ever change the fish style to allow longer lines this should
+    # Thus, if we ever change the fish style to allow longer or shorter lines
-    # be changed (as well as the corresponding clang-format config).
+    # this should be changed (as well as the corresponding .clang-format file).
     - key: LONG_LINE
       value: 100
+
+disable-rules:
+    # A few instances of "useless parentheses" errors are meaningful. Mostly
+    # in the context of the `return` statement. Unfortunately the vast
+    # majority would result in removing parentheses that decreases
+    # readability. So we're going to ignore this warning and rely on humans to
+    # notice when the parentheses are truly not needed.
+    #
+    # Also, some macro expansions, such as FD_SET(), trigger this warning and
+    # we don't want to suppress each of those individually.
+    - UselessParentheses

src/builtin.cpp

@@ -163,7 +163,7 @@ wcstring builtin_help_get(parser_t &parser, io_streams_t &streams, const wchar_t
 /// to an interactive screen, it may be shortened to fit the screen.
 void builtin_print_help(parser_t &parser, io_streams_t &streams, const wchar_t *cmd,
                         output_stream_t &b) {
-    bool is_stderr = (&b == &streams.err);
+    bool is_stderr = &b == &streams.err;
     if (is_stderr) {
         b.append(parser.current_line());
     }

src/color.cpp

@@ -89,8 +89,8 @@ static unsigned long squared_difference(long p1, long p2) {
 static unsigned char convert_color(const unsigned char rgb[3], const uint32_t *colors,
                                    size_t color_count) {
     long r = rgb[0], g = rgb[1], b = rgb[2];
-    unsigned long best_distance = (unsigned long)(-1);
+    unsigned long best_distance = (unsigned long)-1;
-    unsigned char best_index = (unsigned char)(-1);
+    unsigned char best_index = (unsigned char)-1;
     for (unsigned char idx = 0; idx < color_count; idx++) {
         uint32_t color = colors[idx];
         long test_r = (color >> 16) & 0xFF, test_g = (color >> 8) & 0xFF,
@@ -288,7 +288,7 @@ unsigned char rgb_color_t::to_name_index() const {
     } else if (type == type_rgb) {
         return term8_color_for_rgb(data.color.rgb);
     } else {
-        return (unsigned char)(-1);  // this is an error
+        return (unsigned char)-1;  // this is an error
     }
 }
 

src/parse_util.cpp

@@ -595,7 +595,7 @@ static void compute_indents_recursive(const parse_node_tree_t &tree, node_offset
     const bool is_root_job_list = node_type != parent_type && (node_type == symbol_job_list ||
                                                                node_type == symbol_andor_job_list);
     const bool is_root_case_item_list =
-        (node_type == symbol_case_item_list && parent_type != symbol_case_item_list);
+        node_type == symbol_case_item_list && parent_type != symbol_case_item_list;
     if (is_root_job_list || is_root_case_item_list) {
         node_indent += 1;
     }
@@ -845,7 +845,7 @@ void parse_util_expand_variable_error(const wcstring &token, size_t global_token
     // dollar sign.
     assert(errors != NULL);
     assert(dollar_pos < token.size());
-    const bool double_quotes = (token.at(dollar_pos) == VARIABLE_EXPAND_SINGLE);
+    const bool double_quotes = token.at(dollar_pos) == VARIABLE_EXPAND_SINGLE;
     const size_t start_error_count = errors->size();
     const size_t global_dollar_pos = global_token_pos + dollar_pos;
     const size_t global_after_dollar_pos = global_dollar_pos + 1;
@@ -1043,7 +1043,7 @@ parser_test_error_bits_t parse_util_detect_errors_in_argument(const parse_node_t
             switch (unesc.at(idx)) {
                 case VARIABLE_EXPAND:
                 case VARIABLE_EXPAND_SINGLE: {
-                    wchar_t next_char = (idx + 1 < unesc_size ? unesc.at(idx + 1) : L'\0');
+                    wchar_t next_char = idx + 1 < unesc_size ? unesc.at(idx + 1) : L'\0';
 
                     if (next_char != VARIABLE_EXPAND && next_char != VARIABLE_EXPAND_SINGLE &&
                         !wcsvarchr(next_char)) {

src/screen.cpp

@@ -89,7 +89,7 @@ static size_t try_sequence(const char *seq, const wchar_t *str) {
 /// Returns the number of columns left until the next tab stop, given the current cursor postion.
 static size_t next_tab_stop(size_t in) {
     // Assume tab stops every 8 characters if undefined.
-    size_t tab_width = (init_tabs > 0 ? (size_t)init_tabs : 8);
+    size_t tab_width = init_tabs > 0 ? (size_t)init_tabs : 8;
     return ((in / tab_width) + 1) * tab_width;
 }
 
@@ -496,8 +496,8 @@ static void s_move(screen_t *s, data_buffer_t *b, int new_x, int new_y) {
 
     // Use the bulk ('multi') output for cursor movement if it is supported and it would be shorter
     // Note that this is required to avoid some visual glitches in iTerm (issue #1448).
-    bool use_multi = (multi_str != NULL && multi_str[0] != '\0' &&
+    bool use_multi = multi_str != NULL && multi_str[0] != '\0' &&
-                      abs(x_steps) * strlen(str) > strlen(multi_str));
+        abs(x_steps) * strlen(str) > strlen(multi_str);
     if (use_multi) {
         char *multi_param = tparm(multi_str, abs(x_steps));
         writembs(multi_param);
@@ -699,12 +699,12 @@ static void s_update(screen_t *scr, const wchar_t *left_prompt, const wchar_t *r
     for (size_t i = 0; i < scr->desired.line_count(); i++) {
         const line_t &o_line = scr->desired.line(i);
         line_t &s_line = scr->actual.create_line(i);
-        size_t start_pos = (i == 0 ? left_prompt_width : 0);
+        size_t start_pos = i == 0 ? left_prompt_width : 0;
         int current_width = 0;
 
         // If this is the last line, maybe we should clear the screen.
         const bool should_clear_screen_this_line =
-            (need_clear_screen && i + 1 == scr->desired.line_count() && clr_eos != NULL);
+            need_clear_screen && i + 1 == scr->desired.line_count() && clr_eos != NULL;
 
         // Note that skip_remaining is a width, not a character count.
         size_t skip_remaining = start_pos;
@@ -788,7 +788,7 @@ static void s_update(screen_t *scr, const wchar_t *left_prompt, const wchar_t *r
             clear_remainder = true;
         } else {
             int prev_width =
-                (s_line.text.empty() ? 0 : fish_wcswidth(&s_line.text.at(0), s_line.text.size()));
+                s_line.text.empty() ? 0 : fish_wcswidth(&s_line.text.at(0), s_line.text.size());
             clear_remainder = prev_width > current_width;
         }
         if (clear_remainder) {
@@ -842,7 +842,7 @@ static void s_update(screen_t *scr, const wchar_t *left_prompt, const wchar_t *r
 }
 
 /// Returns true if we are using a dumb terminal.
-static bool is_dumb(void) { return (!cursor_up || !cursor_down || !cursor_left || !cursor_right); }
+static bool is_dumb(void) { return !cursor_up || !cursor_down || !cursor_left || !cursor_right; }
 
 struct screen_layout_t {
     // The left prompt that we're going to use.

src/utf8.cpp

@@ -127,9 +127,8 @@ static int __utf8_forbitten(unsigned char octet);
 
 static int __wchar_forbitten(utf8_wchar_t sym) {
     // Surrogate pairs.
-    if (sym >= 0xd800 && sym <= 0xdfff) return (-1);
+    if (sym >= 0xd800 && sym <= 0xdfff) return -1;
-
+    return 0;
-    return (0);
 }
 
 static int __utf8_forbitten(unsigned char octet) {
@@ -138,11 +137,11 @@ static int __utf8_forbitten(unsigned char octet) {
         case 0xc1:
         case 0xf5:
         case 0xff: {
-            return (-1);
+            return -1;
         }
     }
 
-    return (0);
+    return 0;
 }
 
 /// This function translates UTF-8 string into UCS-2 or UCS-4 string (all symbols will be in local
@@ -170,7 +169,7 @@ static size_t utf8_to_wchar_internal(const char *in, size_t insize, utf8_wstring
     utf8_wchar_t high;
     size_t n, total, i, n_bits;
 
-    if (in == NULL || insize == 0) return (0);
+    if (in == NULL || insize == 0) return 0;
 
     if (out_string != NULL) out_string->clear();
 
@@ -179,7 +178,7 @@ static size_t utf8_to_wchar_internal(const char *in, size_t insize, utf8_wstring
     lim = p + insize;
 
     for (; p < lim; p += n) {
-        if (__utf8_forbitten(*p) != 0 && (flags & UTF8_IGNORE_ERROR) == 0) return (0);
+        if (__utf8_forbitten(*p) != 0 && (flags & UTF8_IGNORE_ERROR) == 0) return 0;
 
         // Get number of bytes for one wide character.
         n = 1;  // default: 1 byte. Used when skipping bytes
@@ -201,13 +200,13 @@ static size_t utf8_to_wchar_internal(const char *in, size_t insize, utf8_wstring
             n = 6;
             high = (utf8_wchar_t)(*p & 0x01);
         } else {
-            if ((flags & UTF8_IGNORE_ERROR) == 0) return (0);
+            if ((flags & UTF8_IGNORE_ERROR) == 0) return 0;
             continue;
         }
 
         // Does the sequence header tell us truth about length?
         if (lim - p <= n - 1) {
-            if ((flags & UTF8_IGNORE_ERROR) == 0) return (0);
+            if ((flags & UTF8_IGNORE_ERROR) == 0) return 0;
             n = 1;
             continue;  // skip
         }
@@ -218,7 +217,7 @@ static size_t utf8_to_wchar_internal(const char *in, size_t insize, utf8_wstring
                 if ((p[i] & 0xc0) != _NXT) break;
             }
             if (i != n) {
-                if ((flags & UTF8_IGNORE_ERROR) == 0) return (0);
+                if ((flags & UTF8_IGNORE_ERROR) == 0) return 0;
                 n = 1;
                 continue;  // skip
             }
@@ -256,7 +255,7 @@ static size_t utf8_to_wchar_internal(const char *in, size_t insize, utf8_wstring
         }
     }
 
-    return (total);
+    return total;
 }
 
 /// This function translates UCS-2/4 symbols (given in local machine byte order) into UTF-8 string.
@@ -278,7 +277,7 @@ static size_t wchar_to_utf8_internal(const utf8_wchar_t *in, size_t insize, char
     unsigned char *p, *lim;
     size_t total, n;
 
-    if (in == NULL || insize == 0 || (outsize == 0 && out != NULL)) return (0);
+    if (in == NULL || insize == 0 || (outsize == 0 && out != NULL)) return 0;
 
     w = in;
     wlim = w + insize;
@@ -288,7 +287,7 @@ static size_t wchar_to_utf8_internal(const utf8_wchar_t *in, size_t insize, char
     for (; w < wlim; w++) {
         if (__wchar_forbitten(*w) != 0) {
             if ((flags & UTF8_IGNORE_ERROR) == 0)
-                return (0);
+                return 0;
             else
                 continue;
         }
@@ -297,7 +296,7 @@ static size_t wchar_to_utf8_internal(const utf8_wchar_t *in, size_t insize, char
 
         const int32_t w_wide = *w;
         if (w_wide < 0) {
-            if ((flags & UTF8_IGNORE_ERROR) == 0) return (0);
+            if ((flags & UTF8_IGNORE_ERROR) == 0) return 0;
             continue;
         } else if (w_wide <= 0x0000007f)
             n = 1;
@@ -316,7 +315,7 @@ static size_t wchar_to_utf8_internal(const utf8_wchar_t *in, size_t insize, char
 
         if (out == NULL) continue;
 
-        if (lim - p <= n - 1) return (0); /* no space left */
+        if (lim - p <= n - 1) return 0;  // no space left
 
         // Extract the wchar_t as big-endian. If wchar_t is UCS-16, the first two bytes will be 0.
         unsigned char oc[4];
@@ -376,5 +375,5 @@ static size_t wchar_to_utf8_internal(const utf8_wchar_t *in, size_t insize, char
         p += n;
     }
 
-    return (total);
+    return total;
 }