Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Same issue with two different "Refactor" methods #4356

Closed
daFreeMan opened this issue Sep 6, 2018 · 3 comments
Closed

Same issue with two different "Refactor" methods #4356

daFreeMan opened this issue Sep 6, 2018 · 3 comments
Labels
bug Identifies work items for known bugs feature-refactorings refactoring-rename

Comments

@daFreeMan
Copy link
Contributor

Win10, Excel2016 (desktop), RD build .3751

In the following code, I've attempted both a Refactor|Move closer to usage and a Refactor|Rename on the variable curLen, and I get what appears to be the same error presented in two different ways.

This issue has survived two restarts of Excel and seems very specific to this bit of code. I've been doing clean up for the last several days and have used both Move closer to usage and Rename dozens, if not hundreds of times without any issues all all.

Look for the big comment block about 2/3 of the way down this (very long) Sub (that is in desperate need of several applications of a functioning Refactor|Extract Method):

Private Sub GenerateGraph(ByRef clinicList As ProcessClinic, ByRef report As Worksheet, ByVal DataRange As Range, ByVal projSettings As ProjectSettings)

  Const clinicCol As Long = 1
  Const NPSCol As Long = 2
  Const GoalCol As Long = 3
  Const SATCol As Long = 4
  Const SatSurveyGoalCol As Long = 5
  
  Dim theChart As ChartObject
  Set theChart = report.ChartObjects.Add(20, 20, 1000, 500)
  Dim ser As Series
  With theChart
    Set ser = .Chart.SeriesCollection.NewSeries
    With ser
'@Ignore ObjectVariableNotSet
      .Values = "='" & DataRange.Parent.Name & "'!" & DataRange.Columns(NPSCol).Address(external:=False)
      .AxisGroup = xlPrimary
      .ChartType = xlColumnClustered
      .XValues = "'" & DataRange.Parent.Name & "'!" & DataRange.Columns(clinicCol).Address(external:=False)
      .ApplyDataLabels
      .DataLabels.Format.TextFrame2.TextRange.Font.Bold = msoTrue
    End With
    FormatNPSBars ser, projSettings

    Set ser = .Chart.SeriesCollection.NewSeries
    With ser
'@Ignore ObjectVariableNotSet
      .Values = "='" & DataRange.Parent.Name & "'!" & DataRange.Columns(GoalCol).Address(external:=False)
      .ChartType = xlLine
      With .Format.Line
        .ForeColor.ObjectThemeColor = msoThemeColorAccent1
        .ForeColor.TintAndShade = 0
        .ForeColor.Brightness = -0.25
        .Transparency = 0.6
      End With
      .Name = "NPS Goal"
      Dim NPSGoalLabelLocation As Long
      If .Points.count > 1 Then                           'just in case we ever run for only 1 clinic
        NPSGoalLabelLocation = .Points.count - 1
      Else
        NPSGoalLabelLocation = .Points.count
      End If
      With .Points(NPSGoalLabelLocation)
        .HasDataLabel = True
        .DataLabel.text = "NPS Goal"
        .DataLabel.Position = xlLabelPositionAbove
        .DataLabel.HorizontalAlignment = xlLeft
      End With
    End With

    If Not clinicList.GenerateEOHS Then
      Set ser = .Chart.SeriesCollection.NewSeries
      With ser
        .AxisGroup = xlSecondary
'@Ignore ObjectVariableNotSet
        .Values = "='" & DataRange.Parent.Name & "'!" & DataRange.Columns(SATCol).Address(external:=False)
        .ChartType = xlLine
        .ApplyDataLabels
        .DataLabels.Position = xlLabelPositionBelow
        With .Format.Line
          .ForeColor.ObjectThemeColor = msoThemeColorText1
          .ForeColor.TintAndShade = 0
          .ForeColor.Brightness = 0
          .Transparency = 0
        End With
        .Name = "Patient Satisfaction"
      End With

      Set ser = .Chart.SeriesCollection.NewSeries
      With ser
        .AxisGroup = xlSecondary
'@Ignore ObjectVariableNotSet
.Values = "='" & DataRange.Parent.Name & "'!" & DataRange.Columns(SatSurveyGoalCol).Address(external:=False)
        .ChartType = xlLine
        With .Format.Line
          .ForeColor.ObjectThemeColor = msoThemeColorBackground1
          .ForeColor.TintAndShade = 0
          .ForeColor.Brightness = -0.349999994
          .Transparency = 0
        End With
        .Name = "Satisfaction Goal"
        If .Points.count > 1 Then                           'just in case we ever run for only 1 clinic
          NPSGoalLabelLocation = .Points.count - 1
        Else
          NPSGoalLabelLocation = .Points.count
        End If
        With .Points(NPSGoalLabelLocation)
          .HasDataLabel = True
          .DataLabel.text = "Satisfaction Goal"
          .DataLabel.Position = xlLabelPositionAbove
          .DataLabel.HorizontalAlignment = xlLeft
        End With
      End With
    End If
    
    .Chart.Legend.Position = xlLegendPositionBottom
    .Chart.Legend.LegendEntries(1).Delete                         'hate the hard code, but this is series 1, the NPS scores themselves, being removed from the legend

    .Chart.Axes(xlValue).MaximumScale = 100
    .Chart.Axes(xlCategory).TickLabels.Font.Size = 9
    .Chart.Axes(xlValue).MajorGridlines.Delete
    
    .Placement = xlFreeFloating                                   'prevents the chart from moving/stretching when columns are resized
    
    .Chart.HasTitle = True
    Const MAIN_TITLE_TEXT As String = "Business Solutions NPS"
    Const NPS_TITLE_TEXT As String = "NPS"
    Const DATA_RANGE_TEXT As String = " - Data Collection Range: "
    Const PATIENT_SATISFACTION_TITLE_TEXT As String = "Patient Satisfaction"

    'vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
    'vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
    'error happens when working with this declaration
    Dim curLen As Long
    '^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    '^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    With .Chart.chartTitle
      .text = MAIN_TITLE_TEXT & Chr$(10) & _
              NPS_TITLE_TEXT & DATA_RANGE_TEXT & Format$(clinicList.YtdStartDate, "mmmm d - ") & Format$(clinicList.YtdEndDate, "mmmm d, yyyy") & Chr$(10) & _
              PATIENT_SATISFACTION_TITLE_TEXT & DATA_RANGE_TEXT & Format$(clinicList.YtdStartDate, "mmmm d - ") & Format$(clinicList.YtdEndDate, "mmmm d, yyyy")

      With .Format.TextFrame2.TextRange.Characters(1, Len(MAIN_TITLE_TEXT)).Font
        .Bold = msoTrue
        .Size = 14
      End With
      curLen = InStr(1, .text, Chr$(10))
      With .Format.TextFrame2.TextRange.Characters(curLen, Len(.text) - curLen + 1).Font
        .Bold = msoFalse
        .Italic = msoTrue
        .Size = 10.5
      End With
      With .Format.TextFrame2.TextRange.Characters(curLen, Len(NPS_TITLE_TEXT) + 1).Font
        .Fill.ForeColor.ObjectThemeColor = msoThemeColorText2
        .Fill.ForeColor.Brightness = -0.25
        .Bold = msoTrue
      End With
      curLen = InStr(curLen + 1, .text, Chr$(10))
      With .Format.TextFrame2.TextRange.Characters(curLen, Len(PATIENT_SATISFACTION_TITLE_TEXT) + 1).Font
        .Fill.ForeColor.ObjectThemeColor = msoThemeColorAccent6
        .Fill.ForeColor.Brightness = -0.5
        .Bold = msoTrue
      End With
    End With
  End With

  CreateLabelDef Green, theChart, projSettings
  CreateLabelDef Yellow, theChart, projSettings
  CreateLabelDef Red, theChart, projSettings
  
End Sub

When attempting to Move closer to usage, I get the following lines in the error log:

2018-09-06 07:58:50.6977;DEBUG-2.2.0.3751;Rubberduck.UI.Command.MenuItems.ParentMenus.ParentMenuItemBase;(59242391) Executing click handler for menu item 'Move Closer To &Usage', hash code 25427556;
2018-09-06 07:58:50.7446;ERROR-2.2.0.3751;Rubberduck.UI.Command.Refactorings.RefactorMoveCloserToUsageCommand;System.ArgumentException: replace op boundaries of <ReplaceOp@[@2860,7464:7464='.',<41>,184:11]..[@2869,7504:7509='curLen',<237>,184:51]:"curLen"> overlap with previous <ReplaceOp@[@2860,7464:7464='.',<41>,184:11]..[@2880,7525:7530='curLen',<237>,184:72]:"curLen">
   at Antlr4.Runtime.TokenStreamRewriter.ReduceToSingleOperationPerIndex(IList`1 rewrites)
   at Antlr4.Runtime.TokenStreamRewriter.GetText(String programName, Interval interval)
   at Rubberduck.Parsing.Rewriter.ModuleRewriter.get_IsDirty() in C:\projects\rubberduck\Rubberduck.Parsing\Rewriter\ModuleRewriter.cs:line 27
   at Rubberduck.Parsing.Rewriter.ModuleRewriter.Rewrite() in C:\projects\rubberduck\Rubberduck.Parsing\Rewriter\ModuleRewriter.cs:line 31
   at Rubberduck.Refactorings.MoveCloserToUsage.MoveCloserToUsageRefactoring.MoveCloserToUsage() in C:\projects\rubberduck\Rubberduck.Refactorings\MoveCloserToUsage\MoveCloserToUsageRefactoring.cs:line 118
   at Rubberduck.UI.Command.Refactorings.RefactorMoveCloserToUsageCommand.OnExecute(Object parameter) in C:\projects\rubberduck\Rubberduck.Core\UI\Command\Refactorings\RefactorMoveCloserToUsageCommand.cs:line 49
   at Rubberduck.UI.Command.CommandBase.Execute(Object parameter) in C:\projects\rubberduck\Rubberduck.Core\UI\Command\CommandBase.cs:line 47;System.ArgumentException: replace op boundaries of <ReplaceOp@[@2860,7464:7464='.',<41>,184:11]..[@2869,7504:7509='curLen',<237>,184:51]:"curLen"> overlap with previous <ReplaceOp@[@2860,7464:7464='.',<41>,184:11]..[@2880,7525:7530='curLen',<237>,184:72]:"curLen">
   at Antlr4.Runtime.TokenStreamRewriter.ReduceToSingleOperationPerIndex(IList`1 rewrites)
   at Antlr4.Runtime.TokenStreamRewriter.GetText(String programName, Interval interval)
   at Rubberduck.Parsing.Rewriter.ModuleRewriter.get_IsDirty() in C:\projects\rubberduck\Rubberduck.Parsing\Rewriter\ModuleRewriter.cs:line 27
   at Rubberduck.Parsing.Rewriter.ModuleRewriter.Rewrite() in C:\projects\rubberduck\Rubberduck.Parsing\Rewriter\ModuleRewriter.cs:line 31
   at Rubberduck.Refactorings.MoveCloserToUsage.MoveCloserToUsageRefactoring.MoveCloserToUsage() in C:\projects\rubberduck\Rubberduck.Refactorings\MoveCloserToUsage\MoveCloserToUsageRefactoring.cs:line 118
   at Rubberduck.UI.Command.Refactorings.RefactorMoveCloserToUsageCommand.OnExecute(Object parameter) in C:\projects\rubberduck\Rubberduck.Core\UI\Command\Refactorings\RefactorMoveCloserToUsageCommand.cs:line 49
   at Rubberduck.UI.Command.CommandBase.Execute(Object parameter) in C:\projects\rubberduck\Rubberduck.Core\UI\Command\CommandBase.cs:line 47

Here is the complete and fresh log showing the issue:
RubberduckLog.txt

When attempting to Rename I get this error message:
2018-09-06 07_41_50-microsoft visual basic for applications

@daFreeMan
Copy link
Contributor Author

daFreeMan commented Sep 6, 2018

I just came across another instance in code where Rename is giving an error attempting to rename parameter WS. I tried a couple of different text strings, just to be sure it wasn't somehow related to that - same results.

2018-09-06 11_26_26-microsoft visual basic for applications

Public Sub SortTable(ByVal WS As Worksheet, ByVal TopRow As Long, ByVal BotRow As Long, ByVal LeftCol As Long, ByVal RightCol As Long, ByVal SortCol1 As Long, _
                     Optional ByVal SortCol2 As Long = 0, Optional ByVal column1SortOrder As Long = xlDescending, Optional ByVal column2SortOrder As Long = xlDescending)

  'sort the block
  With WS
    .Sort.SortFields.Clear
    .Sort.SortFields.Add Key:=.Range(.Cells(TopRow, SortCol1), .Cells(BotRow, SortCol1)), SortOn:=xlSortOnValues, Order:=column1SortOrder, DataOption:=xlSortNormal
    If SortCol2 <> 0 Then
      .Sort.SortFields.Add Key:=.Range(.Cells(TopRow, SortCol2), .Cells(BotRow, SortCol2)), SortOn:=xlSortOnValues, Order:=column2SortOrder, DataOption:=xlSortNormal
    End If
    With .Sort
      .SetRange WS.Range(WS.Cells(TopRow, LeftCol), WS.Cells(BotRow, RightCol))
      .Header = xlNo
      .MatchCase = False
      .Orientation = xlTopToBottom
      .SortMethod = xlPinYin
      .Apply
    End With

    FixSortOrderFlags .Range(.Cells(TopRow, LeftCol), .Cells(BotRow, RightCol))
  End With
  
End Sub

This time I actually paid attention to the error message in the dialog box, I'm trying to rename WS, but the error quotes curLen, as in my original issue report.
2018-09-06 13_31_23-microsoft visual basic for applications

@Vogel612 Vogel612 added bug Identifies work items for known bugs feature-refactorings refactoring-rename labels Sep 6, 2018
@Vogel612 Vogel612 added this to ToDo in Semi-automatic bug tracker via automation Sep 6, 2018
@MDoerner
Copy link
Contributor

MDoerner commented Oct 8, 2018

The problem here seems to be that the refactoring tries to rewrite the same tokens multiple times, which is not possible. There are two possible reasons for this: either the refactoring itself does it or there is stale state in the rewriter.

Further investigation is necessary.

@bclothier
Copy link
Contributor

Closing this as it should be already addressed by PR #4465

Semi-automatic bug tracker automation moved this from ToDo to Done Jan 17, 2019
@Vogel612 Vogel612 modified the milestone: 2.4.1 Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Identifies work items for known bugs feature-refactorings refactoring-rename
Projects
Development

No branches or pull requests

4 participants