fix loplugin rewriter source range checking

after
commit 94ab8e4360a2a7a932656e99f718244321d0f923
Date:   Fri Feb 9 15:28:41 2018 +0200
improve loplugin rewriter double source modification detection

Change-Id: Ibf0a64fe4cc3dd6bf5ae16672b3d748a842196e4

3 files changed, 41 insertions, 10 deletions

diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index ef2b7667de85..984c9e13d759 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -397,9 +397,15 @@ bool RewritePlugin::insertText( SourceLocation Loc, StringRef Str, bool InsertAf
     assert( rewriter );
     if (wouldRewriteWorkdir(Loc))
         return false;
+    SourceRange Range(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    if( !handler.checkOverlap( Range ) )
+    {
+        report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", Range.getBegin());
+        return false;
+    }
     if( rewriter->InsertText( Loc, Str, InsertAfter, indentNewLines ))
         return reportEditFailure( Loc );
-    handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    handler.addSourceModification(Range);
     return true;
 }
 
@@ -408,9 +414,15 @@ bool RewritePlugin::insertTextAfter( SourceLocation Loc, StringRef Str )
     assert( rewriter );
     if (wouldRewriteWorkdir(Loc))
         return false;
+    SourceRange Range(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    if( !handler.checkOverlap( Range ) )
+    {
+        report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", Range.getBegin());
+        return false;
+    }
     if( rewriter->InsertTextAfter( Loc, Str ))
         return reportEditFailure( Loc );
-    handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    handler.addSourceModification(Range);
     return true;
 }
 
@@ -419,9 +431,15 @@ bool RewritePlugin::insertTextAfterToken( SourceLocation Loc, StringRef Str )
     assert( rewriter );
     if (wouldRewriteWorkdir(Loc))
         return false;
+    SourceRange Range(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    if( !handler.checkOverlap( Range ) )
+    {
+        report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", Range.getBegin());
+        return false;
+    }
     if( rewriter->InsertTextAfterToken( Loc, Str ))
         return reportEditFailure( Loc );
-    handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    handler.addSourceModification(Range);
     return true;
 }
 
@@ -430,9 +448,15 @@ bool RewritePlugin::insertTextBefore( SourceLocation Loc, StringRef Str )
     assert( rewriter );
     if (wouldRewriteWorkdir(Loc))
         return false;
+    SourceRange Range(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    if( !handler.checkOverlap( Range ) )
+    {
+        report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", Range.getBegin());
+        return false;
+    }
     if( rewriter->InsertTextBefore( Loc, Str ))
         return reportEditFailure( Loc );
-    handler.addSourceModification(SourceRange(Loc, Loc.getLocWithOffset(Str.size())));
+    handler.addSourceModification(Range);
     return true;
 }
 
@@ -457,7 +481,7 @@ bool RewritePlugin::removeText( CharSourceRange range, RewriteOptions opts )
     if( !handler.checkOverlap( range.getAsRange() ) )
     {
         report( DiagnosticsEngine::Warning, "double code removal, possible plugin error", range.getBegin());
-        return true;
+        return false;
     }
     if( opts.flags & RemoveWholeStatement || opts.flags & RemoveAllWhitespace )
     {
@@ -520,7 +544,7 @@ bool RewritePlugin::replaceText( SourceLocation Start, unsigned OrigLength, Stri
     if( OrigLength != 0 && !handler.checkOverlap( Range ) )
     {
         report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", Start );
-        return true;
+        return false;
     }
     if( rewriter->ReplaceText( Start, OrigLength, NewStr ))
         return reportEditFailure( Start );
@@ -538,7 +562,7 @@ bool RewritePlugin::replaceText( SourceRange range, StringRef NewStr )
     if( !handler.checkOverlap( range ) )
     {
         report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", range.getBegin());
-        return true;
+        return false;
     }
     if( rewriter->ReplaceText( range, NewStr ))
         return reportEditFailure( range.getBegin());
@@ -556,7 +580,7 @@ bool RewritePlugin::replaceText( SourceRange range, SourceRange replacementRange
     if( !handler.checkOverlap( range ) )
     {
         report( DiagnosticsEngine::Warning, "double code replacement, possible plugin error", range.getBegin());
-        return true;
+        return false;
     }
     if( rewriter->ReplaceText( range, replacementRange ))
         return reportEditFailure( range.getBegin());
diff --git a/compilerplugins/clang/pluginhandler.cxx b/compilerplugins/clang/pluginhandler.cxx
index 3e78030993be..959941fb72a9 100644
--- a/compilerplugins/clang/pluginhandler.cxx
+++ b/compilerplugins/clang/pluginhandler.cxx
@@ -250,10 +250,17 @@ bool PluginHandler::checkOverlap(SourceRange range)
         if (p1 <= rPair.second && rPair.first <= p2)
             return false;
     }
-    mvModifiedRanges.emplace_back(p1, p2);
     return true;
 }
 
+void PluginHandler::addSourceModification(SourceRange range)
+{
+    SourceManager& SM = compiler.getSourceManager();
+    char const  *p1 = SM.getCharacterData( range.getBegin() );
+    char const *p2 = SM.getCharacterData( range.getEnd() );
+    mvModifiedRanges.emplace_back(p1, p2);
+}
+
 void PluginHandler::HandleTranslationUnit( ASTContext& context )
 {
     if( context.getDiagnostics().hasErrorOccurred())
diff --git a/compilerplugins/clang/pluginhandler.hxx b/compilerplugins/clang/pluginhandler.hxx
index 05e8ce3502c7..890b0cf53c3d 100644
--- a/compilerplugins/clang/pluginhandler.hxx
+++ b/compilerplugins/clang/pluginhandler.hxx
@@ -60,7 +60,7 @@ public:
     // If we overlap with a previous area we modified, we cannot perform this change
     // without corrupting the source
     bool checkOverlap(SourceRange range);
-    bool addSourceModification(SourceRange range);
+    void addSourceModification(SourceRange range);
 private:
     void handleOption( const std::string& option );
     void createPlugins( std::set< std::string > rewriters );