fix: editor diagnostic line number shifting (#3891)

* CodeRange refinement

* update test

* lint fix

Composer/packages/client/__tests__/pages/notifications/useNotifications.test.tsx

@@ -4,6 +4,7 @@
 import * as React from 'react';
 import { RecoilRoot } from 'recoil';
 import { renderHook } from '@bfc/test-utils/lib/hooks';
+import { Range, Position } from '@bfc/shared';
 
 import useNotifications from '../../../src/pages/notifications/useNotifications';
 import {
@@ -37,10 +38,7 @@ const state = {
           Body: '- test12345 ss',
           Entities: [],
           Name: 'test',
-          range: {
-            endLineNumber: 7,
-            startLineNumber: 4,
-          },
+          range: new Range(new Position(4, 0), new Position(7, 14)),
         },
       ],
       diagnostics: [
@@ -64,7 +62,7 @@ const state = {
         {
           body: '- ${add(1,2)}',
           name: 'bar',
-          range: { endLineNumber: 0, startLineNumber: 0 },
+          range: new Range(new Position(0, 0), new Position(2, 14)),
         },
       ],
       diagnostics: [

Composer/packages/client/src/pages/notifications/types.ts

@@ -102,8 +102,8 @@ export class LgNotification extends Notification {
   private findDialogPath(lgFile: LgFile, dialogs: DialogInfo[], diagnostic: Diagnostic) {
     const mappedTemplate = lgFile.templates.find(
       (t) =>
-        get(diagnostic, 'range.start.line') >= get(t, 'range.startLineNumber') &&
-        get(diagnostic, 'range.end.line') <= get(t, 'range.endLineNumber')
+        get(diagnostic, 'range.start.line') >= get(t, 'range.start.line') &&
+        get(diagnostic, 'range.end.line') <= get(t, 'range.end.line')
     );
     if (mappedTemplate && mappedTemplate.name.match(LgNamePattern)) {
       //should navigate to design page

Composer/packages/client/src/recoilModel/parsers/__test__/lgWorker.test.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import { LgFile } from '@bfc/shared';
+import { LgFile, Range, Position } from '@bfc/shared';
 
 import lgWorker from '../lgWorker';
 
@@ -28,7 +28,9 @@ const lgFiles = [
 describe('test lg worker', () => {
   it('get expected parse result', async () => {
     const result: any = await lgWorker.parse('common.en-us', `\r\n# Hello\r\n-hi`, lgFiles);
-    const expected = [{ body: '-hi', name: 'Hello', parameters: [], range: { endLineNumber: 3, startLineNumber: 2 } }];
+    const expected = [
+      { body: '-hi', name: 'Hello', parameters: [], range: new Range(new Position(2, 0), new Position(3, 3)) },
+    ];
     expect(result.templates).toMatchObject(expected);
   });
 });

Composer/packages/client/src/recoilModel/parsers/__test__/luWorker.test.ts

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+import { Range, Position } from '@bfc/shared';
+
 import luWorker from '../luWorker';
 
 jest.mock('./../workers/luParser.worker.ts', () => {
@@ -21,7 +23,9 @@ describe('test lu worker', () => {
     const content = `# Hello
 - hi`;
     const result: any = await luWorker.parse('', content);
-    const expected = [{ Body: '- hi', Entities: [], Name: 'Hello', range: { endLineNumber: 2, startLineNumber: 1 } }];
+    const expected = [
+      { Body: '- hi', Entities: [], Name: 'Hello', range: new Range(new Position(1, 0), new Position(2, 4)) },
+    ];
     expect(result.intents).toMatchObject(expected);
   });
 

Composer/packages/lib/indexers/__tests__/lgIndexer.test.ts

@@ -16,8 +16,8 @@ describe('parse', () => {
 -What's up bro`;
     const { templates }: any = parse(content);
     expect(templates.length).toEqual(2);
-    expect(templates[0].range.startLineNumber).toEqual(1);
-    expect(templates[0].range.endLineNumber).toEqual(3);
+    expect(templates[0].range.start.line).toEqual(1);
+    expect(templates[0].range.end.line).toEqual(3);
     expect(templates[0].parameters).toEqual([]);
     expect(templates[0].name).toEqual('Exit');
     expect(templates[1].name).toEqual('Greeting');

Composer/packages/lib/indexers/__tests__/lgUtil.test.ts

@@ -25,17 +25,13 @@ describe('update lg template', () => {
     expect(templates[0].name).toEqual('Exit');
     expect(templates[0].body).toContain('-Thanks for using todo bot.');
     expect(templates[0].parameters).toEqual([]);
-    expect(templates[0].range).toEqual({
-      startLineNumber: 1,
-      endLineNumber: 3,
-    });
+    expect(templates[0].range?.start.line).toEqual(1);
+    expect(templates[0].range?.end.line).toEqual(3);
     expect(templates[1].name).toEqual('Greeting');
     expect(templates[1].body).toContain(`-What's up bro`);
     expect(templates[1].parameters).toEqual([]);
-    expect(templates[1].range).toEqual({
-      startLineNumber: 4,
-      endLineNumber: 5,
-    });
+    expect(templates[1].range?.start.line).toEqual(4);
+    expect(templates[1].range?.end.line).toEqual(5);
   });
 
   it('should update lg template', () => {
@@ -105,6 +101,10 @@ describe('update lg template', () => {
 -What's up bro`;
     const lgFile = parse('a.lg', content, []);
 
+    expect(lgFile.diagnostics.length).toEqual(1);
+    expect(lgFile.diagnostics[0].range?.start.line).toEqual(2);
+    expect(lgFile.diagnostics[0].range?.end.line).toEqual(2);
+
     const templates0 = Templates.parseText(content).toArray();
     expect(templates0.length).toEqual(2);
     const template = { name: 'Exit', parameters: [], body: '-Bye' };
@@ -168,6 +168,8 @@ describe('check lg template', () => {
     };
     const diags = checkTemplate(template);
     expect(diags).toHaveLength(1);
+    expect(diags[0].range?.start.line).toEqual(2);
+    expect(diags[0].range?.end.line).toEqual(2);
   });
 });
 

Composer/packages/lib/indexers/__tests__/luUtil.test.ts

@@ -2,11 +2,28 @@
 // Licensed under the MIT License.
 import { sectionHandler } from '@microsoft/bf-lu/lib/parser/composerindex';
 
-import { updateIntent, addIntent, removeIntent } from '../src/utils/luUtil';
+import { updateIntent, addIntent, removeIntent, checkSection, parse } from '../src/utils/luUtil';
 import { luIndexer } from '../src/luIndexer';
 
 const { luParser, luSectionTypes } = sectionHandler;
 
+describe('LU Check', () => {
+  const diagnostics1 = checkSection({
+    Name: 'Greeting',
+    Body: `- hi
+- hello`,
+  });
+  expect(diagnostics1.length).toEqual(0);
+
+  const diagnostics2 = checkSection({
+    Name: 'Greeting',
+    Body: `- hi
+hello`,
+  });
+  expect(diagnostics2.length).toEqual(1);
+  expect(diagnostics2[0].range?.start.line).toEqual(3);
+});
+
 describe('LU Section CRUD test', () => {
   const fileContent = `# Greeting
 - hi
@@ -58,6 +75,19 @@ hi
     expect(Sections[2].Errors.length).toEqual(1);
   });
 
+  it('parse section can get diagnostic line number', () => {
+    const luFile = parse(fileId2, fileContentError1);
+    const { intents, diagnostics, content } = luFile;
+
+    expect(content).toEqual(fileContentError1);
+    expect(intents.length).toEqual(3);
+    expect(diagnostics.length).toEqual(2);
+    expect(diagnostics[0].range?.start.line).toEqual(3);
+    expect(diagnostics[0].range?.end.line).toEqual(3);
+    expect(diagnostics[1].range?.start.line).toEqual(10);
+    expect(diagnostics[1].range?.end.line).toEqual(10);
+  });
+
   it('add simpleIntentSection test', () => {
     const intent = {
       Name: 'CheckUnreadEmail',

Composer/packages/lib/indexers/src/utils/diagnosticUtil.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import { Diagnostic, DiagnosticSeverity, Range, Position, CodeRange, LgFile, LuFile } from '@bfc/shared';
+import { Diagnostic, DiagnosticSeverity, Range, Position, LgFile, LuFile } from '@bfc/shared';
 import formatMessage from 'format-message';
 
 export function createSingleMessage(d: Diagnostic): string {
@@ -53,9 +53,9 @@ export function offsetRange(range: Range, offset: number): Range {
   );
 }
 
-export function isDiagnosticWithInRange(diagnostic: Diagnostic, range: CodeRange): boolean {
+export function isDiagnosticWithInRange(diagnostic: Diagnostic, range: Range): boolean {
   if (!diagnostic.range) return false;
-  return diagnostic.range.start.line >= range.startLineNumber && diagnostic.range.end.line <= range.endLineNumber;
+  return diagnostic.range.start.line >= range.start.line && diagnostic.range.end.line <= range.end.line;
 }
 
 export function filterTemplateDiagnostics(file: LgFile, name: string): Diagnostic[] {
@@ -66,7 +66,7 @@ export function filterTemplateDiagnostics(file: LgFile, name: string): Diagnosti
   const filteredDiags = diagnostics.filter((d) => {
     return d.range && isDiagnosticWithInRange(d, range);
   });
-  const offset = range.startLineNumber;
+  const offset = range.start.line;
   return filteredDiags.map((d) => {
     const { range } = d;
     if (range) {
@@ -87,7 +87,7 @@ export function filterSectionDiagnostics(file: LuFile, name: string): Diagnostic
   const filteredDiags = diagnostics.filter((d) => {
     return isDiagnosticWithInRange(d, range);
   });
-  const offset = range.startLineNumber;
+  const offset = range.start.line;
   return filteredDiags.map((d) => {
     const { range } = d;
     if (range) {

Composer/packages/lib/indexers/src/utils/lgUtil.ts

@@ -7,20 +7,13 @@
  *
  */
 
-import { Templates, Diagnostic as LGDiagnostic, ImportResolverDelegate } from 'botbuilder-lg';
+import { Templates, Template, Diagnostic as LGDiagnostic, ImportResolverDelegate } from 'botbuilder-lg';
 import { LgTemplate, importResolverGenerator, TextFile, Diagnostic, Position, Range, LgFile } from '@bfc/shared';
-import get from 'lodash/get';
 import formatMessage from 'format-message';
 import isEmpty from 'lodash/isEmpty';
 
 import { lgIndexer } from '../lgIndexer';
 
-export interface Template {
-  name: string;
-  parameters?: string[];
-  body: string;
-}
-
 // NOTE: LGDiagnostic is defined in PascalCase which should be corrected
 function convertLGDiagnostic(d: LGDiagnostic, source: string): Diagnostic {
   const result = new Diagnostic(d.message, source, d.severity);
@@ -38,10 +31,7 @@ function templateToLgTemplate(templates: Template[]): LgTemplate[] {
       name: t.name,
       body: t.body,
       parameters: t.parameters || [],
-      range: {
-        startLineNumber: get(t, 'sourceRange.range.start.line', 0),
-        endLineNumber: get(t, 'sourceRange.range.end.line', 0),
-      },
+      range: t.sourceRange.range,
     };
   });
 }

Composer/packages/lib/indexers/src/utils/luUtil.ts

@@ -22,7 +22,7 @@ const NEWLINE = '\r\n';
 // when new add a section in inline editor, the section haven't exist on file context, to make suggestion/validation possiable here mock one.
 export const PlaceHolderSectionName = formatMessage(`_NewSectionPlaceHolderSectionName`);
 
-export function convertLuDiagnostic(d: any, source: string): Diagnostic {
+export function convertLuDiagnostic(d: any, source: string, offset = 0): Diagnostic {
   const severityMap = {
     ERROR: DiagnosticSeverity.Error,
     WARN: DiagnosticSeverity.Warning,
@@ -31,8 +31,10 @@ export function convertLuDiagnostic(d: any, source: string): Diagnostic {
   };
   const result = new Diagnostic(d.Message, source, severityMap[d.Severity]);
 
-  const start: Position = d.Range ? new Position(d.Range.Start.Line, d.Range.Start.Character) : new Position(0, 0);
-  const end: Position = d.Range ? new Position(d.Range.End.Line, d.Range.End.Character) : new Position(0, 0);
+  const start: Position = d.Range
+    ? new Position(d.Range.Start.Line - offset, d.Range.Start.Character)
+    : new Position(0, 0);
+  const end: Position = d.Range ? new Position(d.Range.End.Line - offset, d.Range.End.Character) : new Position(0, 0);
   result.range = new Range(start, end);
 
   return result;
@@ -44,20 +46,20 @@ export function convertLuParseResultToLuFile(id = '', resource): LuFile {
   const intents: LuIntentSection[] = [];
   Sections.forEach((section) => {
     const { Name, Body, SectionType } = section;
-    const range = {
-      startLineNumber: get(section, 'Range.Start.Line', 0),
-      endLineNumber: get(section, 'Range.End.Line', 0),
-    };
+    const range = new Range(
+      new Position(get(section, 'Range.Start.Line', 0), get(section, 'Range.Start.Character', 0)),
+      new Position(get(section, 'Range.End.Line', 0), get(section, 'Range.End.Character', 0))
+    );
     if (SectionType === LuSectionTypes.SIMPLEINTENTSECTION) {
       const Entities = section.Entities.map(({ Name }) => Name);
       intents.push({ Name, Body, Entities, range });
     } else if (SectionType === LuSectionTypes.NESTEDINTENTSECTION) {
       const Children = section.SimpleIntentSections.map((subSection) => {
         const { Name, Body } = subSection;
-        const range = {
-          startLineNumber: get(subSection, 'Range.Start.Line', 0),
-          endLineNumber: get(subSection, 'Range.End.Line', 0),
-        };
+        const range = new Range(
+          new Position(get(section, 'Range.Start.Line', 0), get(subSection, 'Range.Start.Character', 0)),
+          new Position(get(section, 'Range.End.Line', 0), get(subSection, 'Range.End.Character', 0))
+        );
         const Entities = subSection.Entities.map(({ Name }) => Name);
         return { Name, Body, Entities, range };
       });
@@ -132,7 +134,7 @@ export function checkSection(intent: LuIntentSection, enableSections = true): Di
   const text = textFromIntent(intent, 1, enableSections);
   const result = luParser.parse(text);
   const { Errors } = result;
-  return Errors.map((e) => convertLuDiagnostic(e, ''));
+  return Errors.map((e) => convertLuDiagnostic(e, '', enableSections ? 1 : 0));
 }
 
 export function checkIsSingleSection(intent: LuIntentSection, enableSections = true): boolean {

Composer/packages/lib/shared/src/types/indexers.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import { Diagnostic } from './diagnostic';
+import { Diagnostic, Range } from './diagnostic';
 import { IIntentTrigger } from './dialogUtils';
 
 import { DialogSetting } from './index';
@@ -77,7 +77,7 @@ export interface LuIntentSection {
   Body: string;
   Entities?: LuEntity[];
   Children?: LuIntentSection[];
-  range?: CodeRange;
+  range?: Range;
 }
 
 export interface LuParsed {
@@ -117,16 +117,12 @@ export interface QnAFile {
   qnaSections: QnASection[];
   [key: string]: any;
 }
-export interface CodeRange {
-  startLineNumber: number;
-  endLineNumber: number;
-}
 
 export interface LgTemplate {
   name: string;
   body: string;
   parameters: string[];
-  range?: CodeRange;
+  range?: Range;
 }
 
 export interface LgParsed {

Composer/packages/tools/language-servers/language-generation/src/LGServer.ts

@@ -720,7 +720,7 @@ export class LGServer {
 
       // filter diagnostics belong to this template.
       const lgDiagnostics = filterTemplateDiagnostics(lgFile, templateId);
-      const lspDiagnostics = convertDiagnostics(lgDiagnostics, document, 1);
+      const lspDiagnostics = convertDiagnostics(lgDiagnostics, document);
       this.sendDiagnostics(document, lspDiagnostics);
       return;
     }

Composer/packages/tools/language-servers/language-understanding/src/LUServer.ts

@@ -174,8 +174,7 @@ export class LUServer {
       }
 
       const id = fileId || uri;
-      const { intents: sections, diagnostics: bfIndexerDiags } = parse(content, id);
-      const diagnostics = convertDiagnostics(bfIndexerDiags, document);
+      const { intents: sections, diagnostics } = parse(content, id);
 
       return { sections, diagnostics, content };
     };

Composer/packages/tools/language-servers/language-understanding/src/utils.ts

@@ -53,12 +53,11 @@ export function generageDiagnostic(message: string, severity: DiagnosticSeverity
   };
 }
 
-// if section, offset +1 to exclude #IntentName
 export function convertDiagnostics(lgDiags: BFDiagnostic[] = [], document: TextDocument, offset = 0): Diagnostic[] {
   const diagnostics: Diagnostic[] = [];
   const defaultRange = Range.create(Position.create(0, 0), Position.create(0, 0));
   lgDiags.forEach((diag) => {
-    const range = diag.range ? offsetRange(diag.range, offset) : defaultRange;
+    const range = diag.range ? offsetRange(diag.range, 1 + offset) : defaultRange;
     const diagnostic: Diagnostic = {
       severity: convertSeverity(diag.severity),
       range,