Assert on valid order of which ids in ranges on SfxItemSet creation

This allows to make sure we actually use sorted which ranges,
and then it's safe to call SfxItemSet::MergeRange when needed.

Also this change relaxes the previous requirement that ranges
must be separated by at least one; this allows to have adjacent
ranges, like in

RES_FRMATR_BEGIN, RES_FRMATR_END-1,
RES_GRFATR_BEGIN, RES_GRFATR_END-1,

where RES_FRMATR_END is equal to RES_GRFATR_BEGIN. Allowing this
makes possible to (1) self-document the ranges, so it's clear
which ranges are included; and (2) be safe in case when these
constants would change, so that the one merged range would not
unexpectedly contain everything inserted between RES_FRMATR_END
and RES_GRFATR_BEGIN.

Change-Id: Iaad0f099b85059b3aa318a347aa7fbd3f6d455c7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/116909
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

12 files changed, 54 insertions, 77 deletions

diff --git a/sw/source/core/bastyp/init.cxx b/sw/source/core/bastyp/init.cxx
index 1acc4bfe4f59..8bc7296b250f 100644
--- a/sw/source/core/bastyp/init.cxx
+++ b/sw/source/core/bastyp/init.cxx
@@ -147,9 +147,9 @@ sal_uInt16 const aBreakSetRange[] = {
 // list attributes ( RES_PARATR_LIST_BEGIN - RES_PARATR_LIST_END ) are not
 // included in the paragraph style's itemset.
 sal_uInt16 const aTextFormatCollSetRange[] = {
-    RES_FRMATR_BEGIN, RES_FRMATR_END-1,
     RES_CHRATR_BEGIN, RES_CHRATR_END-1,
     RES_PARATR_BEGIN, RES_PARATR_END-1,
+    RES_FRMATR_BEGIN, RES_FRMATR_END-1,
     RES_UNKNOWNATR_BEGIN, RES_UNKNOWNATR_END-1,
 
     // FillAttribute support
@@ -168,10 +168,10 @@ sal_uInt16 const aGrfFormatCollSetRange[] = {
 
 // AttrSet range for TextNode
 sal_uInt16 const aTextNodeSetRange[] = {
-    RES_FRMATR_BEGIN, RES_FRMATR_END-1,
     RES_CHRATR_BEGIN, RES_CHRATR_END-1,
     RES_PARATR_BEGIN, RES_PARATR_END-1,
     RES_PARATR_LIST_BEGIN, RES_PARATR_LIST_END-1,
+    RES_FRMATR_BEGIN, RES_FRMATR_END-1,
     RES_UNKNOWNATR_BEGIN, RES_UNKNOWNATR_END-1,
 
     // FillAttribute support (paragraph FillStyle)
@@ -191,43 +191,43 @@ sal_uInt16 const aNoTextNodeSetRange[] = {
 sal_uInt16 const aTableSetRange[] = {
     RES_FILL_ORDER,     RES_FRM_SIZE,
     RES_LR_SPACE,       RES_BREAK,
-    RES_BACKGROUND,     RES_SHADOW,
     RES_HORI_ORIENT,    RES_HORI_ORIENT,
+    RES_BACKGROUND,     RES_SHADOW,
     RES_KEEP,           RES_KEEP,
     RES_LAYOUT_SPLIT,   RES_LAYOUT_SPLIT,
     RES_FRAMEDIR,       RES_FRAMEDIR,
     // #i29550#
     RES_COLLAPSING_BORDERS, RES_COLLAPSING_BORDERS,
     // <-- collapsing
-    RES_UNKNOWNATR_BEGIN, RES_UNKNOWNATR_END-1,
     RES_FRMATR_GRABBAG, RES_FRMATR_GRABBAG,
+    RES_UNKNOWNATR_BEGIN, RES_UNKNOWNATR_END-1,
     0
 };
 
 sal_uInt16 const aTableLineSetRange[] = {
     RES_FILL_ORDER,     RES_FRM_SIZE,
     // HasTextChangesOnly
-    RES_PRINT,          RES_PRINT,
     RES_LR_SPACE,       RES_UL_SPACE,
-    RES_BACKGROUND,     RES_SHADOW,
-    RES_ROW_SPLIT,      RES_ROW_SPLIT,
+    RES_PRINT,          RES_PRINT,
     RES_PROTECT,        RES_PROTECT,
     RES_VERT_ORIENT,    RES_VERT_ORIENT,
-    RES_UNKNOWNATR_BEGIN, RES_UNKNOWNATR_END-1,
+    RES_BACKGROUND,     RES_SHADOW,
+    RES_ROW_SPLIT,      RES_ROW_SPLIT,
     RES_FRMATR_GRABBAG, RES_FRMATR_GRABBAG,
+    RES_UNKNOWNATR_BEGIN, RES_UNKNOWNATR_END-1,
     0
 };
 
 sal_uInt16 const aTableBoxSetRange[] = {
     RES_FILL_ORDER,     RES_FRM_SIZE,
     RES_LR_SPACE,       RES_UL_SPACE,
-    RES_BACKGROUND,     RES_SHADOW,
     RES_PROTECT,        RES_PROTECT,
     RES_VERT_ORIENT,    RES_VERT_ORIENT,
+    RES_BACKGROUND,     RES_SHADOW,
     RES_FRAMEDIR,       RES_FRAMEDIR,
+    RES_FRMATR_GRABBAG, RES_FRMATR_GRABBAG,
     RES_BOXATR_BEGIN,   RES_BOXATR_END-1,
     RES_UNKNOWNATR_BEGIN, RES_UNKNOWNATR_END-1,
-    RES_FRMATR_GRABBAG, RES_FRMATR_GRABBAG,
     0
 };
 
diff --git a/sw/source/core/doc/docnew.cxx b/sw/source/core/doc/docnew.cxx
index a674beb9fde6..ae2f24a92a8a 100644
--- a/sw/source/core/doc/docnew.cxx
+++ b/sw/source/core/doc/docnew.cxx
@@ -845,10 +845,10 @@ void SwDoc::ReplaceDefaults(const SwDoc& rSource)
     // copy property defaults
     const sal_uInt16 aRangeOfDefaults[] =
     {
-        RES_FRMATR_BEGIN, RES_FRMATR_END-1,
         RES_CHRATR_BEGIN, RES_CHRATR_END-1,
         RES_PARATR_BEGIN, RES_PARATR_END-1,
         RES_PARATR_LIST_BEGIN, RES_PARATR_LIST_END-1,
+        RES_FRMATR_BEGIN, RES_FRMATR_END-1,
         RES_UNKNOWNATR_BEGIN, RES_UNKNOWNATR_END-1,
         XATTR_START, XATTR_END-1,
         0
diff --git a/sw/source/core/docnode/ndtbl.cxx b/sw/source/core/docnode/ndtbl.cxx
index 0cc141a2b6c9..a17ce9b3e041 100644
--- a/sw/source/core/docnode/ndtbl.cxx
+++ b/sw/source/core/docnode/ndtbl.cxx
@@ -3051,15 +3051,11 @@ void sw_BoxSetSplitBoxFormats( SwTableBox* pBox, SwCollectTableLineBoxes* pSplPa
         }
         else
         {
-            sal_uInt16 const aTableSplitBoxSetRange[] {
-                RES_LR_SPACE,       RES_UL_SPACE,
-                RES_BACKGROUND,     RES_SHADOW,
-                RES_PROTECT,        RES_PROTECT,
-                RES_VERT_ORIENT,    RES_VERT_ORIENT,
-                0 };
-
             SfxItemSet aTmpSet( pFormat->GetDoc()->GetAttrPool(),
-                                aTableSplitBoxSetRange );
+                                svl::Items<RES_LR_SPACE,    RES_UL_SPACE,
+                                           RES_PROTECT,     RES_PROTECT,
+                                           RES_VERT_ORIENT, RES_VERT_ORIENT,
+                                           RES_BACKGROUND,  RES_SHADOW>{} );
             aTmpSet.Put( pFormat->GetAttrSet() );
             if( aTmpSet.Count() )
                 pBox->ClaimFrameFormat()->SetFormatAttr( aTmpSet );
diff --git a/sw/source/core/fields/expfld.cxx b/sw/source/core/fields/expfld.cxx
index 6ee39cf4d8a0..8b6917094f89 100644
--- a/sw/source/core/fields/expfld.cxx
+++ b/sw/source/core/fields/expfld.cxx
@@ -985,13 +985,13 @@ sal_Int32 SwGetExpField::GetReferenceTextPos( const SwFormatField& rFormat, SwDo
 
         static const sal_uInt16 nIds[] =
         {
-            RES_CHRATR_LANGUAGE, RES_CHRATR_LANGUAGE,
             RES_CHRATR_FONT, RES_CHRATR_FONT,
-            RES_CHRATR_CJK_LANGUAGE, RES_CHRATR_CJK_LANGUAGE,
+            RES_CHRATR_LANGUAGE, RES_CHRATR_LANGUAGE,
             RES_CHRATR_CJK_FONT, RES_CHRATR_CJK_FONT,
-            RES_CHRATR_CTL_LANGUAGE, RES_CHRATR_CTL_LANGUAGE,
+            RES_CHRATR_CJK_LANGUAGE, RES_CHRATR_CJK_LANGUAGE,
             RES_CHRATR_CTL_FONT, RES_CHRATR_CTL_FONT,
-            0, 0
+            RES_CHRATR_CTL_LANGUAGE, RES_CHRATR_CTL_LANGUAGE,
+            0
         };
         SwAttrSet aSet(rDoc.GetAttrPool(), nIds);
         rTextNode.GetParaAttr(aSet, nRet, nRet+1);
diff --git a/sw/source/core/txtnode/txtedt.cxx b/sw/source/core/txtnode/txtedt.cxx
index 6e64a06c570d..8913fbd94810 100644
--- a/sw/source/core/txtnode/txtedt.cxx
+++ b/sw/source/core/txtnode/txtedt.cxx
@@ -1068,15 +1068,10 @@ void SwTextNode::SetLanguageAndFont( const SwPaM &rPaM,
     LanguageType nLang, sal_uInt16 nLangWhichId,
     const vcl::Font *pFont,  sal_uInt16 nFontWhichId )
 {
-    sal_uInt16 aRanges[] = {
-            nLangWhichId, nLangWhichId,
-            nFontWhichId, nFontWhichId,
-            0, 0, 0 };
-    if (!pFont)
-        aRanges[2] = aRanges[3] = 0;    // clear entries with font WhichId
-
     SwEditShell *pEditShell = GetDoc().GetEditShell();
-    SfxItemSet aSet( pEditShell->GetAttrPool(), aRanges );
+    SfxItemSet aSet(pEditShell->GetAttrPool(), { { nLangWhichId, nLangWhichId } });
+    if (pFont)
+        aSet.MergeRange(nFontWhichId, nFontWhichId); // Keep it sorted
     aSet.Put( SvxLanguageItem( nLang, nLangWhichId ) );
 
     OSL_ENSURE( pFont, "target font missing?" );
diff --git a/sw/source/core/unocore/unocrsrhelper.cxx b/sw/source/core/unocore/unocrsrhelper.cxx
index 58d5dbf6b11e..1c9f9d4b28db 100644
--- a/sw/source/core/unocore/unocrsrhelper.cxx
+++ b/sw/source/core/unocore/unocrsrhelper.cxx
@@ -1292,7 +1292,7 @@ void makeRedline( SwPaM const & rPaM,
             SwDoc& rDoc = rPaM.GetDoc();
 
             // Build set of attributes we want to fetch
-            std::vector<sal_uInt16> aWhichPairs;
+            std::vector<std::pair<sal_uInt16, sal_uInt16>> aWhichPairs;
             std::vector<SfxItemPropertyMapEntry const*> aEntries;
             std::vector<uno::Any> aValues;
             aEntries.reserve(aRevertProperties.getLength());
@@ -1314,15 +1314,12 @@ void makeRedline( SwPaM const & rPaM,
                 }
                 else if (rPropertyName == "NumberingRules")
                 {
-                    aWhichPairs.push_back(RES_PARATR_NUMRULE);
-                    aWhichPairs.push_back(RES_PARATR_NUMRULE);
+                    aWhichPairs.emplace_back(RES_PARATR_NUMRULE, RES_PARATR_NUMRULE);
                     nNumId = aEntries.size();
                 }
                 else
                 {
-                    // FIXME: we should have some nice way of merging ranges surely ?
-                    aWhichPairs.push_back(pEntry->nWID);
-                    aWhichPairs.push_back(pEntry->nWID);
+                    aWhichPairs.emplace_back(pEntry->nWID, pEntry->nWID);
                     if (rPropertyName == "ParaStyleName")
                         nStyleId = aEntries.size();
                 }
@@ -1334,8 +1331,9 @@ void makeRedline( SwPaM const & rPaM,
             {
                 sal_uInt16 nStylePoolId = USHRT_MAX;
                 OUString sParaStyleName;
-                aWhichPairs.push_back(0); // terminate
-                SfxItemSet aItemSet(rDoc.GetAttrPool(), aWhichPairs.data());
+                SfxItemSet aItemSet(rDoc.GetAttrPool(), nullptr);
+                for (const auto& [nWhich1, nWhich2] : aWhichPairs)
+                    aItemSet.MergeRange(nWhich1, nWhich2);
 
                 for (size_t i = 0; i < aEntries.size(); ++i)
                 {
diff --git a/sw/source/uibase/app/docshini.cxx b/sw/source/uibase/app/docshini.cxx
index e5d43853e938..284d4a84afdd 100644
--- a/sw/source/uibase/app/docshini.cxx
+++ b/sw/source/uibase/app/docshini.cxx
@@ -621,11 +621,11 @@ void SwDocShell::SubInitNew()
     bool bWeb = dynamic_cast< const SwWebDocShell *>( this ) !=  nullptr;
 
     sal_uInt16 nRange[] =   {
-        RES_PARATR_ADJUST, RES_PARATR_ADJUST,
         RES_CHRATR_COLOR, RES_CHRATR_COLOR,
         RES_CHRATR_LANGUAGE, RES_CHRATR_LANGUAGE,
         RES_CHRATR_CJK_LANGUAGE, RES_CHRATR_CJK_LANGUAGE,
         RES_CHRATR_CTL_LANGUAGE, RES_CHRATR_CTL_LANGUAGE,
+        RES_PARATR_ADJUST, RES_PARATR_ADJUST,
         0, 0, 0  };
     if(!bWeb)
     {
diff --git a/sw/source/uibase/lingu/hhcwrp.cxx b/sw/source/uibase/lingu/hhcwrp.cxx
index 498fb484c472..372737149744 100644
--- a/sw/source/uibase/lingu/hhcwrp.cxx
+++ b/sw/source/uibase/lingu/hhcwrp.cxx
@@ -467,12 +467,9 @@ void SwHHCWrapper::ReplaceUnit(
             OSL_ENSURE( GetTargetLanguage() == LANGUAGE_CHINESE_SIMPLIFIED || GetTargetLanguage() == LANGUAGE_CHINESE_TRADITIONAL,
                     "SwHHCWrapper::ReplaceUnit : unexpected target language" );
 
-            sal_uInt16 const aRanges[] {
-                    RES_CHRATR_CJK_LANGUAGE, RES_CHRATR_CJK_LANGUAGE,
+            SfxItemSet aSet( m_rWrtShell.GetAttrPool(), svl::Items<
                     RES_CHRATR_CJK_FONT,     RES_CHRATR_CJK_FONT,
-                    0, 0, 0  };
-
-            SfxItemSet aSet( m_rWrtShell.GetAttrPool(), aRanges );
+                    RES_CHRATR_CJK_LANGUAGE, RES_CHRATR_CJK_LANGUAGE>{} );
             if (pNewUnitLanguage)
             {
                 aSet.Put( SvxLanguageItem( *pNewUnitLanguage, RES_CHRATR_CJK_LANGUAGE ) );
diff --git a/sw/source/uibase/shells/tabsh.cxx b/sw/source/uibase/shells/tabsh.cxx
index 067f78c04368..cd231be93192 100644
--- a/sw/source/uibase/shells/tabsh.cxx
+++ b/sw/source/uibase/shells/tabsh.cxx
@@ -96,29 +96,29 @@ void SwTableShell::InitInterface_Impl()
 
 const sal_uInt16 aUITableAttrRange[] =
 {
-    XATTR_FILL_FIRST,               XATTR_FILL_LAST,
-    FN_PARAM_TABLE_NAME,            FN_PARAM_TABLE_NAME,
-    FN_PARAM_TABLE_HEADLINE,        FN_PARAM_TABLE_HEADLINE,
-    FN_PARAM_TABLE_SPACE,           FN_PARAM_TABLE_SPACE,
-    FN_TABLE_REP,                   FN_TABLE_REP,
-    SID_RULER_BORDERS,              SID_RULER_BORDERS,
     RES_LR_SPACE,                   RES_UL_SPACE,
-    SID_ATTR_BORDER_INNER,          SID_ATTR_BORDER_SHADOW,
-    RES_BOX,                        RES_SHADOW,
-    RES_BACKGROUND,                 RES_BACKGROUND,
-    SID_BACKGRND_DESTINATION,       SID_BACKGRND_DESTINATION,
-    SID_HTML_MODE,                  SID_HTML_MODE,
-    SID_ATTR_BRUSH_ROW,             SID_ATTR_BRUSH_TABLE,
     RES_PAGEDESC,                   RES_BREAK,
+    RES_BACKGROUND,                 RES_BACKGROUND,
+    RES_BOX,                        RES_SHADOW,
     RES_KEEP,                       RES_KEEP,
     RES_LAYOUT_SPLIT,               RES_LAYOUT_SPLIT,
-    FN_TABLE_SET_VERT_ALIGN,        FN_TABLE_SET_VERT_ALIGN,
     RES_FRAMEDIR,                   RES_FRAMEDIR,
     RES_ROW_SPLIT,                  RES_ROW_SPLIT,
-    FN_TABLE_BOX_TEXTORIENTATION,   FN_TABLE_BOX_TEXTORIENTATION,
 // #i29550#
     RES_COLLAPSING_BORDERS,         RES_COLLAPSING_BORDERS,
 // <-- collapsing borders
+    XATTR_FILL_FIRST,               XATTR_FILL_LAST,
+    SID_ATTR_BORDER_INNER,          SID_ATTR_BORDER_SHADOW,
+    SID_RULER_BORDERS,              SID_RULER_BORDERS,
+    SID_ATTR_BRUSH_ROW,             SID_ATTR_BRUSH_TABLE, // ??? This is very strange range
+//  SID_BACKGRND_DESTINATION,       SID_BACKGRND_DESTINATION, // included into above
+//  SID_HTML_MODE,                  SID_HTML_MODE, // included into above
+    FN_TABLE_REP,                   FN_TABLE_REP,
+    FN_TABLE_SET_VERT_ALIGN,        FN_TABLE_SET_VERT_ALIGN,
+    FN_TABLE_BOX_TEXTORIENTATION,   FN_TABLE_BOX_TEXTORIENTATION,
+    FN_PARAM_TABLE_NAME,            FN_PARAM_TABLE_NAME,
+    FN_PARAM_TABLE_HEADLINE,        FN_PARAM_TABLE_HEADLINE,
+    FN_PARAM_TABLE_SPACE,           FN_PARAM_TABLE_SPACE,
     0
 };
 
diff --git a/sw/source/uibase/shells/textsh.cxx b/sw/source/uibase/shells/textsh.cxx
index 8300a91295ca..12f6f8d9bdad 100644
--- a/sw/source/uibase/shells/textsh.cxx
+++ b/sw/source/uibase/shells/textsh.cxx
@@ -837,20 +837,15 @@ SwTextShell::~SwTextShell()
 
 SfxItemSet SwTextShell::CreateInsertFrameItemSet(SwFlyFrameAttrMgr& rMgr)
 {
-    static const sal_uInt16 aFrameAttrRange[] =
-    {
+    SfxItemSet aSet(GetPool(), svl::Items<
         RES_FRMATR_BEGIN,       RES_FRMATR_END-1,
+        XATTR_FILL_FIRST,       XATTR_FILL_LAST, // tdf#95003
         SID_ATTR_BORDER_INNER,  SID_ATTR_BORDER_INNER,
-        FN_GET_PRINT_AREA,      FN_GET_PRINT_AREA,
         SID_ATTR_PAGE_SIZE,     SID_ATTR_PAGE_SIZE,
-        FN_SET_FRM_NAME,        FN_SET_FRM_NAME,
-        SID_HTML_MODE,          SID_HTML_MODE,
         SID_COLOR_TABLE,        SID_PATTERN_LIST,
-        XATTR_FILL_FIRST,       XATTR_FILL_LAST, // tdf#95003
-        0
-    };
-
-    SfxItemSet aSet(GetPool(), aFrameAttrRange );
+        SID_HTML_MODE,          SID_HTML_MODE,
+        FN_GET_PRINT_AREA,      FN_GET_PRINT_AREA,
+        FN_SET_FRM_NAME,        FN_SET_FRM_NAME>{});
     aSet.Put(SfxUInt16Item(SID_HTML_MODE, ::GetHtmlMode(GetView().GetDocShell())));
 
     // For the Area tab page.
diff --git a/sw/source/uibase/uiview/viewsrch.cxx b/sw/source/uibase/uiview/viewsrch.cxx
index 614c5606da7a..911d2cedd1eb 100644
--- a/sw/source/uibase/uiview/viewsrch.cxx
+++ b/sw/source/uibase/uiview/viewsrch.cxx
@@ -741,9 +741,9 @@ sal_uLong SwView::FUNC_Search( const SwSearchOptions& rOptions )
     m_pWrtShell->SttSelect();
 
     static const sal_uInt16 aSearchAttrRange[] = {
-        RES_FRMATR_BEGIN, RES_FRMATR_END-1,
         RES_CHRATR_BEGIN, RES_CHRATR_END-1,
         RES_PARATR_BEGIN, RES_PARATR_END-1,
+        RES_FRMATR_BEGIN, RES_FRMATR_END-1,
         SID_ATTR_PARA_MODEL, SID_ATTR_PARA_KEEP,
         0 };
 
diff --git a/sw/source/uibase/uno/SwXDocumentSettings.cxx b/sw/source/uibase/uno/SwXDocumentSettings.cxx
index 26a2e8285300..98e71d8e09a8 100644
--- a/sw/source/uibase/uno/SwXDocumentSettings.cxx
+++ b/sw/source/uibase/uno/SwXDocumentSettings.cxx
@@ -451,15 +451,11 @@ void SwXDocumentSettings::_setSingleValue( const comphelper::PropertyInfo & rInf
                 SvMemoryStream aStream (aSequence.getArray(), nSize,
                                         StreamMode::READ );
                 aStream.Seek ( STREAM_SEEK_TO_BEGIN );
-                static sal_uInt16 const nRange[] =
-                {
-                    FN_PARAM_ADDPRINTER, FN_PARAM_ADDPRINTER,
-                    SID_HTML_MODE,  SID_HTML_MODE,
+                auto pItemSet = std::make_unique<SfxItemSet>( mpDoc->GetAttrPool(), svl::Items<
                     SID_PRINTER_NOTFOUND_WARN, SID_PRINTER_NOTFOUND_WARN,
                     SID_PRINTER_CHANGESTODOC, SID_PRINTER_CHANGESTODOC,
-                    0
-                };
-                auto pItemSet = std::make_unique<SfxItemSet>( mpDoc->GetAttrPool(), nRange );
+                    SID_HTML_MODE,  SID_HTML_MODE,
+                    FN_PARAM_ADDPRINTER, FN_PARAM_ADDPRINTER>{} );
                 VclPtr<SfxPrinter> pPrinter = SfxPrinter::Create ( aStream, std::move(pItemSet) );
                 assert (! pPrinter->isDisposed() );
                 // set printer only once; in _postSetValues