make the Color constructors explicitly specify transparency

to reduce the churn, we leave the existing constructor in place,
and add a clang plugin to detect when the value passed to the
existing constructor may contain transparency/alpha data.

i.e. we leave expressions like Color(0xffffff) alone, but
warn about any non-constant expression, and any expression
like Color(0xff000000)

Change-Id: Id2ce58e08882d9b7bd0b9f88eca97359dcdbcc8c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/109362
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

8 files changed, 58 insertions, 18 deletions

diff --git a/include/oox/helper/helper.hxx b/include/oox/helper/helper.hxx
index da28c88b9e4b..63718ca0ebed 100644
--- a/include/oox/helper/helper.hxx
+++ b/include/oox/helper/helper.hxx
@@ -78,7 +78,7 @@ const sal_uInt8 WINDOWS_CHARSET_EASTERN     = 238;
 const sal_uInt8 WINDOWS_CHARSET_OEM         = 255;
 
 
-const ::Color API_RGB_TRANSPARENT         (0xffffffff); ///< Transparent color for API calls.
+const ::Color API_RGB_TRANSPARENT         (ColorTransparency, 0xffffffff); ///< Transparent color for API calls.
 const sal_uInt32 UNSIGNED_RGB_TRANSPARENT = static_cast<sal_uInt32>(-1); ///< Transparent color for unsigned int32 places.
 const ::Color API_RGB_BLACK               (0x000000);  ///< Black color for API calls.
 const ::Color API_RGB_GRAY                (0x808080);  ///< Gray color for API calls.
diff --git a/include/sax/tools/converter.hxx b/include/sax/tools/converter.hxx
index 0d3d68ba301c..88cea1c33ab7 100644
--- a/include/sax/tools/converter.hxx
+++ b/include/sax/tools/converter.hxx
@@ -123,7 +123,15 @@ public:
     {
         sal_Int32 n(rColor);
         bool b = convertColor( n, rValue );
-        if (b) rColor = n;
+        if (b) rColor = Color(ColorTransparency, n);
+        return b;
+    }
+    static bool convertColor( ::Color& rColor,
+                              std::string_view rValue )
+    {
+        sal_Int32 n(rColor);
+        bool b = convertColor( n, rValue );
+        if (b) rColor = Color(ColorTransparency, n);
         return b;
     }
 
diff --git a/include/svx/ColorSets.hxx b/include/svx/ColorSets.hxx
index 3b347ede0690..8b4a035a1857 100644
--- a/include/svx/ColorSets.hxx
+++ b/include/svx/ColorSets.hxx
@@ -30,7 +30,7 @@ public:
 
     void add(sal_uInt32 nIndex, sal_uInt32 aColorData)
     {
-        maColors[nIndex] = Color(aColorData);
+        maColors[nIndex] = Color(ColorTransparency, aColorData);
     }
 
     const OUString& getName() const
diff --git a/include/svx/colorwindow.hxx b/include/svx/colorwindow.hxx
index 05917b2dfcfc..136d7621c0e7 100644
--- a/include/svx/colorwindow.hxx
+++ b/include/svx/colorwindow.hxx
@@ -44,7 +44,7 @@ public:
 
 class Button;
 
-#define COL_NONE_COLOR    ::Color(0x80, 0xFF, 0xFF, 0xFF)
+#define COL_NONE_COLOR    ::Color(ColorTransparency, 0x80, 0xFF, 0xFF, 0xFF)
 
 class SvxColorToolBoxControl;
 
diff --git a/include/tools/color.hxx b/include/tools/color.hxx
index aca6b0ed79b7..3226184ca117 100644
--- a/include/tools/color.hxx
+++ b/include/tools/color.hxx
@@ -40,6 +40,10 @@ constexpr sal_uInt8 ColorChannelMerge(sal_uInt8 nDst, sal_uInt8 nSrc, sal_uInt8
 
 }
 
+/** used to deliberately select the right constructor */
+enum ColorTransparencyTag { ColorTransparency = 0 };
+enum ColorAlphaTag { ColorAlpha = 0 };
+
 // Color
 
 class SAL_WARN_UNUSED TOOLS_DLLPUBLIC Color
@@ -70,21 +74,37 @@ public:
         : mValue(0) // black
     {}
 
-    constexpr Color(sal_uInt32 nColor)
+    constexpr Color(const sal_uInt32 nColor)
         : mValue(nColor)
-    {}
+    {
+        assert(nColor <= 0xffffff && "don't pass transparency to this constructor, use the Color(ColorTransparencyTag,...) or Color(ColorAlphaTag,...) constructor to make it explicit");
+    }
 
-    constexpr Color(sal_uInt8 nTransparency, sal_uInt8 nRed, sal_uInt8 nGreen, sal_uInt8 nBlue)
+    constexpr Color(enum ColorTransparencyTag, sal_uInt32 nColor)
+        : mValue(nColor)
+    {
+    }
+
+    constexpr Color(enum ColorAlphaTag, sal_uInt32 nColor)
+        : mValue((nColor & 0xffffff) | (255 - (nColor >> 24)))
+    {
+    }
+
+    constexpr Color(enum ColorTransparencyTag, sal_uInt8 nTransparency, sal_uInt8 nRed, sal_uInt8 nGreen, sal_uInt8 nBlue)
         : mValue(sal_uInt32(nBlue) | (sal_uInt32(nGreen) << 8) | (sal_uInt32(nRed) << 16) | (sal_uInt32(nTransparency) << 24))
     {}
 
+    constexpr Color(enum ColorAlphaTag, sal_uInt8 nAlpha, sal_uInt8 nRed, sal_uInt8 nGreen, sal_uInt8 nBlue)
+        : Color(ColorTransparency, 255 - nAlpha, nRed, nGreen, nBlue)
+    {}
+
     constexpr Color(sal_uInt8 nRed, sal_uInt8 nGreen, sal_uInt8 nBlue)
-        : Color(0, nRed, nGreen, nBlue)
+        : Color(ColorTransparency, 0, nRed, nGreen, nBlue)
     {}
 
     // constructor to create a tools-Color from ::basegfx::BColor
     explicit Color(const basegfx::BColor& rBColor)
-        : Color(0,
+        : Color(ColorTransparency, 0,
                 sal_uInt8(std::lround(rBColor.getRed() * 255.0)),
                 sal_uInt8(std::lround(rBColor.getGreen() * 255.0)),
                 sal_uInt8(std::lround(rBColor.getBlue() * 255.0)))
@@ -394,7 +414,7 @@ inline bool operator >>=( const css::uno::Any & rAny, Color & value )
   sal_Int32 nTmp = {}; // spurious -Werror=maybe-uninitialized
   if (!(rAny >>= nTmp))
       return false;
-  value = Color(nTmp);
+  value = Color(ColorTransparency, nTmp);
   return true;
 }
 
@@ -413,7 +433,7 @@ namespace com::sun::star::uno {
 
 // Test compile time conversion of Color to sal_uInt32
 
-static_assert (sal_uInt32(Color(0x00, 0x12, 0x34, 0x56)) == 0x00123456);
+static_assert (sal_uInt32(Color(ColorTransparency, 0x00, 0x12, 0x34, 0x56)) == 0x00123456);
 static_assert (sal_uInt32(Color(0x12, 0x34, 0x56)) == 0x00123456);
 
 // Color types
@@ -437,8 +457,8 @@ constexpr ::Color COL_LIGHTMAGENTA            ( 0xFF, 0x00, 0xFF );
 constexpr ::Color COL_LIGHTGRAYBLUE           ( 0xE0, 0xE0, 0xFF );
 constexpr ::Color COL_YELLOW                  ( 0xFF, 0xFF, 0x00 );
 constexpr ::Color COL_WHITE                   ( 0xFF, 0xFF, 0xFF );
-constexpr ::Color COL_TRANSPARENT             ( 0xFF, 0xFF, 0xFF, 0xFF );
-constexpr ::Color COL_AUTO                    ( 0xFF, 0xFF, 0xFF, 0xFF );
+constexpr ::Color COL_TRANSPARENT             ( ColorTransparency, 0xFF, 0xFF, 0xFF, 0xFF );
+constexpr ::Color COL_AUTO                    ( ColorTransparency, 0xFF, 0xFF, 0xFF, 0xFF );
 constexpr ::Color COL_AUTHOR1_DARK            ( 198,  146,   0 );
 constexpr ::Color COL_AUTHOR1_NORMAL          ( 255,  255, 158 );
 constexpr ::Color COL_AUTHOR1_LIGHT           ( 255,  255, 195 );
diff --git a/include/vcl/BitmapColor.hxx b/include/vcl/BitmapColor.hxx
index 634ae6902d27..b5364b4d146f 100644
--- a/include/vcl/BitmapColor.hxx
+++ b/include/vcl/BitmapColor.hxx
@@ -27,7 +27,9 @@ class VCL_DLLPUBLIC BitmapColor final : public Color
 {
 public:
     inline              BitmapColor();
-    constexpr           BitmapColor( sal_uInt8 cRed, sal_uInt8 cGreen, sal_uInt8 cBlue, sal_uInt8 cAlpha = 0 );
+    constexpr           BitmapColor( sal_uInt8 cRed, sal_uInt8 cGreen, sal_uInt8 cBlue );
+    constexpr           BitmapColor( ColorTransparencyTag, sal_uInt8 cRed, sal_uInt8 cGreen, sal_uInt8 cBlue, sal_uInt8 cTransparency );
+    constexpr           BitmapColor( ColorAlphaTag, sal_uInt8 cRed, sal_uInt8 cGreen, sal_uInt8 cBlue, sal_uInt8 cAlpha );
 
     inline              BitmapColor( const Color& rColor );
     explicit inline     BitmapColor( sal_uInt8 cIndex );
@@ -45,8 +47,18 @@ inline BitmapColor::BitmapColor( const Color& rColor )
 {
 }
 
-constexpr BitmapColor::BitmapColor(sal_uInt8 cRed, sal_uInt8 cGreen, sal_uInt8 cBlue, sal_uInt8 cAlpha)
-    : Color(cAlpha, cRed, cGreen, cBlue)
+constexpr BitmapColor::BitmapColor(sal_uInt8 cRed, sal_uInt8 cGreen, sal_uInt8 cBlue)
+    : Color(cRed, cGreen, cBlue)
+{
+}
+
+constexpr BitmapColor::BitmapColor(ColorTransparencyTag, sal_uInt8 cRed, sal_uInt8 cGreen, sal_uInt8 cBlue, sal_uInt8 cTransparency)
+    : Color(ColorTransparency, cTransparency, cRed, cGreen, cBlue)
+{
+}
+
+constexpr BitmapColor::BitmapColor(ColorAlphaTag, sal_uInt8 cRed, sal_uInt8 cGreen, sal_uInt8 cBlue, sal_uInt8 cAlpha)
+    : Color(ColorAlpha, cAlpha, cRed, cGreen, cBlue)
 {
 }
 
diff --git a/include/vcl/RawBitmap.hxx b/include/vcl/RawBitmap.hxx
index 79cef7c47e00..d3dcb642cf6c 100644
--- a/include/vcl/RawBitmap.hxx
+++ b/include/vcl/RawBitmap.hxx
@@ -55,7 +55,7 @@ public:
         if (mnBitCount == 24)
             return Color(mpData[p], mpData[p + 1], mpData[p + 2]);
         else
-            return Color(mpData[p + 3], mpData[p], mpData[p + 1], mpData[p + 2]);
+            return Color(ColorTransparency, mpData[p + 3], mpData[p], mpData[p + 1], mpData[p + 2]);
     }
     // so we don't accidentally leave any code in that uses palette color indexes
     void SetPixel(tools::Long nY, tools::Long nX, BitmapColor nColor) = delete;
diff --git a/include/vcl/salgtype.hxx b/include/vcl/salgtype.hxx
index 902170555526..1ab62fbdb6c7 100644
--- a/include/vcl/salgtype.hxx
+++ b/include/vcl/salgtype.hxx
@@ -34,7 +34,7 @@ enum class DeviceFormat {
 #endif
                         };
 
-constexpr ::Color SALCOLOR_NONE ( 0xFF, 0xFF, 0xFF, 0xFF );
+constexpr ::Color SALCOLOR_NONE ( ColorTransparency, 0xFF, 0xFF, 0xFF, 0xFF );
 
 struct SalTwoRect
 {