fidley / ABAPQuickFix

ABAP Quick Fix
https://abapblog.com
MIT License
42 stars 9 forks source link

QuickFix invocation causes BadLocationException in Report - no Proposals shown (or not all) #65

Open lc-leuc opened 1 month ago

lc-leuc commented 1 month ago

Observation

In Reports and also in FM Quickfix proposals are not shown. In debug mode a BadLocationException can be observed in AbapStatement initialization.

Apparently endOfStatement is negative and causes substring(.) to fail.

How to reproduce

Several small test reports show this behavior. For example see this example report (minial). Place curson on data and press Ctrl-1.

report z_test.
   data(temp) = 1.

@fd1899 has reported this odd behavior in ABAPQuickFixS4Conversion/#6 for other reports. The problem occurs with the ABAPQuickFixS4Conversion plugin installes as well as without the plugin.

Versions

Stacktrace

org.eclipse.jface.text.BadLocationException
    at org.eclipse.jface.text.AbstractDocument.getChar(AbstractDocument.java:774)
    at org.eclipse.core.internal.filebuffers.SynchronizableDocument.getChar(SynchronizableDocument.java:141)
    at com.abapblog.adt.quickfix.assist.syntax.codeParser.AbapStatement.<init>(AbapStatement.java:25)
    at com.abapblog.adt.quickfix.assist.syntax.codeParser.AbapCodeReader.createStatement(AbapCodeReader.java:46)
    at com.abapblog.adt.quickfix.assist.syntax.codeParser.AbapCodeReader.<init>(AbapCodeReader.java:34)
    at com.abapblog.adt.quickfix.assist.syntax.codeParser.AbapCodeReader.getInstance(AbapCodeReader.java:55)
    at com.abapblog.adt.quickfix.assist.syntax.statements.StatementsAssistProcessor.computeQuickAssistProposals(StatementsAssistProcessor.java:130)
    at com.sap.adt.tools.abapsource.ui.sources.QuickAssistProcessorManager.computeQuickAssistProposals(QuickAssistProcessorManager.java:58)
    at org.eclipse.jface.text.quickassist.QuickAssistAssistant$ContentAssistProcessor.computeCompletionProposals(QuickAssistAssistant.java:71)
    at org.eclipse.jface.text.contentassist.ContentAssistant$2.lambda$0(ContentAssistant.java:2064)
    at java.base/java.util.Collections$SingletonSet.forEach(Collections.java:5125)
    at org.eclipse.jface.text.contentassist.ContentAssistant$2.run(ContentAssistant.java:2063)
    at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:47)
    at org.eclipse.jface.text.contentassist.ContentAssistant.computeCompletionProposals(ContentAssistant.java:2060)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup.computeProposals(CompletionProposalPopup.java:577)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup.lambda$0(CompletionProposalPopup.java:507)
    at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:67)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup.showProposals(CompletionProposalPopup.java:502)
    at org.eclipse.jface.text.contentassist.ContentAssistant.showPossibleCompletions(ContentAssistant.java:1874)
    at org.eclipse.jface.text.quickassist.QuickAssistAssistant.showPossibleQuickAssists(QuickAssistAssistant.java:113)
    at org.eclipse.jface.text.source.SourceViewer.doOperation(SourceViewer.java:1005)
    at org.eclipse.jface.text.source.projection.ProjectionViewer.doOperation(ProjectionViewer.java:1468)
    at com.sap.adt.tools.abapsource.ui.sources.editors.AdtProjectionViewer.doOperation(AdtProjectionViewer.java:243)
    at com.sap.adt.refactoring.ui.internal.quickfix.AbapQuickfixProcessor$AsyncQuickfixCalculatorJob.retriggerContentAssist(AbapQuickfixProcessor.java:754)
    at com.sap.adt.util.ui.contentassist.AsyncContentAssistProposalCalculatorJob.lambda$0(AsyncContentAssistProposalCalculatorJob.java:74)
    at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
    at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)
    at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4047)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3663)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1151)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1042)
    at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:152)
    at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:639)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
    at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:546)
    at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173)
    at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
    at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:208)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:143)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:109)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:439)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:271)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:651)
    at org.eclipse.equinox.launcher.Main.basicRun(Main.java:588)
    at org.eclipse.equinox.launcher.Main.run(Main.java:1459)
    at org.eclipse.equinox.launcher.Main.main(Main.java:1432)
Range [1, 0) out of bounds for length 41
fidley commented 1 month ago

Thanks for the info. I'll check it soon

fidley commented 1 month ago

please update the plugin and check again

FD1899 commented 1 month ago

Thank you guys! Now it seems to work perfect. :-)

lc-leuc commented 1 month ago

Hi Łukasz,

thanks for the quick response. BTW: thanks for the nice talk on the AbapConf.

Unfortunately, your changes did not resolve the odd behavior within reports (completely).

First Observation

Observed in minimal report (add a line break).

I get a message Range [33, -1) out of bounds for length 35, caused by line 134 in StatementsAssistProcessor.computeQuickAssistProposals().

Obviously, this is caused by the initialization of DataSortByNameAll.

Second Observation

Observed in minimal report (without line break).

Causes BadLocationException (new source), which in turn causes the proposals to be empty.

Within other Report (expected)

grafik

In Minimal Report (see above - not expected)

grafik

Cause - Stack Trace (differs from the one above)

org.eclipse.jface.text.BadLocationException
    at org.eclipse.jface.text.ListLineTracker.getLineOffset(ListLineTracker.java:206)
    at org.eclipse.jface.text.AbstractLineTracker.getLineOffset(AbstractLineTracker.java:252)
    at org.eclipse.jface.text.AbstractDocument.getLineOffset(AbstractDocument.java:868)
    at org.eclipse.core.internal.filebuffers.SynchronizableDocument.getLineOffset(SynchronizableDocument.java:323)
    at com.abapblog.adt.quickfix.assist.syntax.statements.sort.AbstractDataSortByName.getSpaces(AbstractDataSortByName.java:54)
    at com.abapblog.adt.quickfix.assist.syntax.statements.sort.AbstractDataSortByName.<init>(AbstractDataSortByName.java:40)
    at com.abapblog.adt.quickfix.assist.syntax.statements.sort.DataSortByNameAll.<init>(DataSortByNameAll.java:18)
    at com.abapblog.adt.quickfix.assist.syntax.statements.StatementsAssistProcessor.createAssistsList(StatementsAssistProcessor.java:222)
    at com.abapblog.adt.quickfix.assist.syntax.statements.StatementsAssistProcessor.computeQuickAssistProposals(StatementsAssistProcessor.java:131)
    at com.sap.adt.tools.abapsource.ui.sources.QuickAssistProcessorManager.computeQuickAssistProposals(QuickAssistProcessorManager.java:58)
    at org.eclipse.jface.text.quickassist.QuickAssistAssistant$ContentAssistProcessor.computeCompletionProposals(QuickAssistAssistant.java:71)
    at org.eclipse.jface.text.contentassist.ContentAssistant$2.lambda$0(ContentAssistant.java:2064)
    at java.base/java.util.Collections$SingletonSet.forEach(Collections.java:5125)
    at org.eclipse.jface.text.contentassist.ContentAssistant$2.run(ContentAssistant.java:2063)
    at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:47)
    at org.eclipse.jface.text.contentassist.ContentAssistant.computeCompletionProposals(ContentAssistant.java:2060)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup.computeProposals(CompletionProposalPopup.java:577)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup.lambda$0(CompletionProposalPopup.java:507)
    at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:67)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup.showProposals(CompletionProposalPopup.java:502)
    at org.eclipse.jface.text.contentassist.ContentAssistant.showPossibleCompletions(ContentAssistant.java:1874)
    at org.eclipse.jface.text.quickassist.QuickAssistAssistant.showPossibleQuickAssists(QuickAssistAssistant.java:113)
    at org.eclipse.jface.text.source.SourceViewer.doOperation(SourceViewer.java:1005)
    at org.eclipse.jface.text.source.projection.ProjectionViewer.doOperation(ProjectionViewer.java:1468)
    at com.sap.adt.tools.abapsource.ui.sources.editors.AdtProjectionViewer.doOperation(AdtProjectionViewer.java:243)
    at com.sap.adt.refactoring.ui.internal.quickfix.AbapQuickfixProcessor$AsyncQuickfixCalculatorJob.retriggerContentAssist(AbapQuickfixProcessor.java:754)
    at com.sap.adt.util.ui.contentassist.AsyncContentAssistProposalCalculatorJob.lambda$0(AsyncContentAssistProposalCalculatorJob.java:74)
    at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
    at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)
    at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4047)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3663)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1151)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1042)
    at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:152)
    at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:639)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
    at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:546)
    at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173)
    at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
    at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:208)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:143)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:109)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:439)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:271)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:651)
    at org.eclipse.equinox.launcher.Main.basicRun(Main.java:588)
    at org.eclipse.equinox.launcher.Main.run(Main.java:1459)
    at org.eclipse.equinox.launcher.Main.main(Main.java:1432)

Additional Observation

Sort single / all Data removes statements instead of sorting. (Also observed in classes.)

Before

grafik

After

grafik

fidley commented 1 month ago

Hi Lawrence, I will work on this today evening. Seems my last updates were solving one bug, and creating next one.. Cheers

fidley commented 1 month ago

67

fidley commented 1 month ago

Please try now.

fidley commented 1 month ago

@lc-leuc did the latest changes helped?

lc-leuc commented 1 month ago

Hello @fidley, please excuse the delay. After some tests it seems that the example seem to work. However, now I get odd behavior with my implementations.

It seems that the original statement is correct, the replacement is also correct but the original text is not removed completely from the code. Thus leaving a tail (something like the last ten characters or so) of code after the newly inserted code.

Additionally, I noticed that sort all data statements now adds a line to the code (minor) and that simple replacements (like 'eq' -> '=') removes all indentation. I am still investigating...

fidley commented 1 month ago

I've changed a bit how AbapStatment class is working, added there few method to correctly grab the beginning of statement and inline comment at the end. Maybe this has done some harm to your code?

lc-leuc commented 1 month ago

Yes, I noticed. However, my replacement is still correct. The resulting effect in editor is not.

I have isolated another minimal example.

report zsql_test.
    select single *
      from mara
      into @data(tmara)
      where mtart = 'FERT'.

Recognized original statement is correct:

select single *
      from mara
      into @data(tmara)
      where mtart = 'FERT'

Statement boundaries are 23 and 108 - also correct.

Replacement (return of changedCode (of IAssist) - still correct:


    select *
      from mara
      where mtart = 'FERT'
      order by primary key
      into @data(tmara)
      up to 1 rows.
    endselect

However the result is:

grafik

Undo reveils that the end of the statement is not correctly recognized: start at 17; length 85 (expected 91)

grafik

The delta is the lenght of the leading text (23-17 == 91-85). The culprit is probably to be found here:

grafik

dfec3fcb

Changing ths back fixes the problem.

fidley commented 1 month ago

Ok, now fully understand the issue. When you have the getStartOfReplace you use (as I previously) the getBeginOfStatement method. image. As this lead to reading also inline comments of the previous line, I've adapted the code to get "real" beginning of the current statement and I've put this into getBeginOfStatementReplace method. image If you update the getStartOfReplace() then it should work correctly for you again.

fidley commented 1 month ago

I need to add some descriptions of the methods and need to invest in the Unit Tests here, for the sake of all of us :D