Merge branch 'main' into rmohamme.fix-lint-for-ipynb

rashedmyt · rashedmyt · commit 0c28417f8e19 · 2024-09-25T10:35:58.000+05:30
diff --git a/README.md b/README.md
@@ -28,6 +28,13 @@ MATLAB language server supports these editors by installing the corresponding ex
 
 ### Unreleased
 
+### 1.2.6
+Release date: 2024-09-20
+
+Fixed:
+* Patches CVE-2024-43788
+* Resolves issue preventing code navigation and variable renaming for variables followed by a matrix operation (e.g. `x.^2`)
+
 ### 1.2.5
 Release date: 2024-08-16
 
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "matlab-language-server",
-  "version": "1.2.5",
+  "version": "1.2.6",
   "description": "Language Server for MATLAB code",
   "main": "./src/index.ts",
   "bin": "./out/index.js",
@@ -23,6 +23,7 @@
   "devDependencies": {
     "@types/mocha": "^10.0.6",
     "@types/node": "^18.7.18",
+    "@types/sinon": "^17.0.3",
     "@types/which": "^2.0.1",
     "@types/yargs": "^17.0.12",
     "@typescript-eslint/eslint-plugin": "^5.36.1",
@@ -34,6 +35,7 @@
     "eslint-plugin-promise": "^6.0.1",
     "mocha": "^10.4.0",
     "node-loader": "^2.0.0",
+    "sinon": "^18.0.0",
     "ts-loader": "^9.4.1",
     "ts-node": "^10.9.2",
     "typescript": "^4.8.3",
diff --git a/src/ClientConnection.ts b/src/ClientConnection.ts
@@ -4,7 +4,7 @@ import { _Connection, createConnection, ProposedFeatures } from "vscode-language
 export type Connection = _Connection
 
 export default class ClientConnection {
-    private static connection: Connection
+    private static connection: Connection | undefined
 
     /**
      * Retrieves the connection to the client. If no connection currently exists,
@@ -26,7 +26,15 @@ export default class ClientConnection {
      *
      * @param connection The connection object to set
      */
-    public static setConnection (connection: Connection): void {
+    public static _setConnection (connection: Connection): void {
         ClientConnection.connection = connection
     }
+
+    /**
+     * Clears the current connection.
+     * This API is primarily meant for testing purposes.
+     */
+    public static _clearConnection (): void {
+        ClientConnection.connection = undefined
+    }
 }
diff --git a/src/lifecycle/MatlabCommunicationManager.d.ts b/src/lifecycle/MatlabCommunicationManager.d.ts
@@ -1,4 +1,5 @@
 /// <reference types="node" />
+/// <reference types="node" />
 import { ChildProcess } from 'child_process';
 declare const Faye: any;
 declare type Client = typeof Faye.Client;
diff --git a/src/lifecycle/MatlabCommunicationManager.js b/src/lifecycle/MatlabCommunicationManager.js
diff --git a/src/logging/Logger.ts b/src/logging/Logger.ts
@@ -16,7 +16,7 @@ class Logger {
     private readonly languageServerLogFile: string
     private readonly matlabLogFile: string
 
-    private readonly console: RemoteConsole
+    private console?: RemoteConsole
 
     constructor () {
         // Create Log Directory
@@ -32,9 +32,10 @@ class Logger {
         // Get name of log file
         this.languageServerLogFile = path.join(this.logDir, SERVER_LOG)
         this.matlabLogFile = path.join(this.logDir, MATLAB_LOG)
+    }
 
-        // Get log console
-        this.console = ClientConnection.getConnection().console
+    public initialize (console: RemoteConsole): void {
+        this.console = console
         
         this.log(`Log Directory: ${this.logDir}`)
     }
diff --git a/src/providers/formatting/FormatSupportProvider.ts b/src/providers/formatting/FormatSupportProvider.ts
@@ -17,8 +17,8 @@ interface FormatDocumentResponse {
  * include formatting a range witin the documemt.
  */
 class FormatSupportProvider {
-    private readonly REQUEST_CHANNEL = '/matlabls/formatDocument/request'
-    private readonly RESPONSE_CHANNEL = '/matlabls/formatDocument/response'
+    readonly REQUEST_CHANNEL = '/matlabls/formatDocument/request'
+    readonly RESPONSE_CHANNEL = '/matlabls/formatDocument/response'
 
     constructor (private matlabLifecycleManager: MatlabLifecycleManager) {}
 
diff --git a/src/server.ts b/src/server.ts
@@ -30,6 +30,9 @@ export async function startServer () {
     // Create a connection for the server
     const connection = ClientConnection.getConnection()
 
+    // Initialize Logger
+    Logger.initialize(connection.console)
+
     // Instantiate services
     const pathResolver = new PathResolver()
     const matlabLifecycleManager = new MatlabLifecycleManager()
diff --git a/src/utils/ExpressionUtils.ts b/src/utils/ExpressionUtils.ts
@@ -4,6 +4,8 @@ import { TextDocument } from 'vscode-languageserver-textdocument'
 import { Position } from 'vscode-languageserver'
 import { getTextOnLine } from './TextDocumentUtils'
 
+const DOTTED_IDENTIFIER_REGEX = /\b(?:[a-zA-Z][\w]*)(?:\.[a-zA-Z][\w]*)*\b/ // Matches a word followed by optional dotted words
+
 /**
  * Represents a code expression, either a single identifier or a dotted expression.
  * For example, "plot" or "pkg.Class.func".
@@ -82,7 +84,6 @@ export function getExpressionAtPosition (textDocument: TextDocument, position: P
  * @returns An object containing the string identifier at the position, as well as the column number at which the identifier starts.
  */
 function getIdentifierAtPosition (textDocument: TextDocument, position: Position): { identifier: string, start: number } {
-    const DOTTED_IDENTIFIER_REGEX = /[\w.]+/
     let lineText = getTextOnLine(textDocument, position.line)
 
     const result = {
@@ -94,12 +95,19 @@ function getIdentifierAtPosition (textDocument: TextDocument, position: Position
     let offset = 0
 
     while (matchResults != null) {
-        if (matchResults.index == null || matchResults.index > position.character) {
-            // Already passed the cursor - no match found
+        if (matchResults.index == null) {
+            // No result found
             break
         }
 
         const startChar = offset + matchResults.index
+
+        if (startChar > position.character) {
+            // Passed the cursor - no match found
+            break
+        }
+
+        
         if (startChar + matchResults[0].length >= position.character) {
             // Found overlapping identifier
             result.identifier = matchResults[0]
diff --git a/tests/mocks/Connection.mock.ts b/tests/mocks/Connection.mock.ts
@@ -0,0 +1,16 @@
+import sinon from 'sinon'
+
+export default function getMockConnection (): any {
+    const mockConnection = {
+        console: {
+            connection: {},
+            error: sinon.stub(),
+            warn: sinon.stub(),
+            info: sinon.stub(),
+            log: sinon.stub()
+        },
+        sendNotification: sinon.stub()
+    }
+
+    return mockConnection
+}
diff --git a/tests/providers/formatting/FormatSupportProvider.test.ts b/tests/providers/formatting/FormatSupportProvider.test.ts
@@ -0,0 +1,99 @@
+import assert from 'assert'
+import sinon from 'sinon'
+
+import FormatSupportProvider from '../../../src/providers/formatting/FormatSupportProvider'
+import MatlabLifecycleManager from '../../../src/lifecycle/MatlabLifecycleManager'
+import ClientConnection from '../../../src/ClientConnection'
+
+import { TextDocument } from 'vscode-languageserver-textdocument'
+import { _Connection, DocumentFormattingParams, Position, Range, TextDocuments, TextEdit } from 'vscode-languageserver'
+import getMockConnection from '../../mocks/Connection.mock'
+
+describe('FormatSupportProvider', () => {
+    let formatSupportProvider: FormatSupportProvider
+    let matlabLifecycleManager: MatlabLifecycleManager
+    let documentManager: TextDocuments<TextDocument>
+    let mockMatlabConnection: any
+    let mockTextDocument: TextDocument
+
+    before(() => {
+        ClientConnection._setConnection(getMockConnection())
+    })
+
+    after(() => {
+        ClientConnection._clearConnection()
+    })
+    
+    describe('#handleDocumentFormatRequest', () => {
+        // Because the actual formatting logic occurs in MATLAB, the actual value of these
+        // params are not tested in this file. So, define static params for each test.
+        const mockParams = {
+            textDocument: { uri: 'file:///test.m' },
+            options: { insertSpaces: true, tabSize: 4 }
+        } as DocumentFormattingParams
+
+        beforeEach(() => {
+            matlabLifecycleManager = new MatlabLifecycleManager()
+            formatSupportProvider = new FormatSupportProvider(matlabLifecycleManager)
+            documentManager = new TextDocuments(TextDocument)
+            mockMatlabConnection = {
+                getChannelId: sinon.stub().returns('test-channel'),
+                subscribe: sinon.stub(),
+                unsubscribe: sinon.stub(),
+                publish: sinon.stub()
+            }
+            mockTextDocument = TextDocument.create('file:///test.m', 'matlab', 1, 'function y = test(x)\ny = x + 1;\nend')
+
+            sinon.stub(matlabLifecycleManager, 'getMatlabConnection').returns(mockMatlabConnection)
+            sinon.stub(documentManager, 'get').returns(mockTextDocument)
+        })
+
+        afterEach(() => {
+            sinon.restore()
+        })
+        
+        it('should return null if no document to format', async () => {
+            // Return undefined text document
+            (documentManager.get as sinon.SinonStub).returns(undefined)
+
+            const res = await formatSupportProvider.handleDocumentFormatRequest(mockParams, documentManager)
+
+            assert.equal(res, null, 'Result should be null when there is no document')
+        })
+
+        it('should return empty array if no MATLAB connection', async () => {
+            // Return null MATLAB connection
+            (matlabLifecycleManager.getMatlabConnection as sinon.SinonStub).resolves(null)
+
+            const res = await formatSupportProvider.handleDocumentFormatRequest(mockParams, documentManager)
+
+            assert.deepEqual(res, [], 'Result should be [] when there is no connection')
+        })
+
+        it('should handle successful formatting', async () => {
+            const formattedCode = 'function y = test(x)\n    y = x + 1;\nend'
+            const expectedEdit = TextEdit.replace(
+                Range.create(Position.create(0, 0), Position.create(2, 3)),
+                formattedCode
+            )
+
+            mockMatlabConnection.subscribe.callsFake((channel: string, callback: any) => {
+                setTimeout(() => {
+                    callback({ data: formattedCode })
+                }, 0)
+                return 'subscription-id'
+            })
+
+            const result = await formatSupportProvider.handleDocumentFormatRequest(mockParams, documentManager)
+
+            assert.deepStrictEqual(result, [expectedEdit])
+            sinon.assert.calledOnce(mockMatlabConnection.publish)
+            sinon.assert.calledWith(mockMatlabConnection.publish, formatSupportProvider.REQUEST_CHANNEL, {
+                data: mockTextDocument.getText(),
+                insertSpaces: true,
+                tabSize: 4,
+                channelId: 'test-channel'
+            })
+        })
+    })
+})
diff --git a/tests/utils/ExpressionUtils.test.ts b/tests/utils/ExpressionUtils.test.ts
@@ -0,0 +1,84 @@
+import assert from 'assert'
+import { Position } from 'vscode-languageserver'
+import { TextDocument } from 'vscode-languageserver-textdocument'
+import * as ExpressionUtils from '../../src/utils/ExpressionUtils'
+
+describe('ExpressionUtils', () => {
+    describe('#getExpressionAtPosition', () => {
+        it('should return null for empty line', () => {
+            const doc = TextDocument.create('', '', 1, '')
+            const position = Position.create(0, 0)
+            const result = ExpressionUtils.getExpressionAtPosition(doc, position)
+            assert.strictEqual(result, null)
+        })
+
+        it('should return correct expression for single identifier', () => {
+            const doc = TextDocument.create('', '', 1, 'someVariable')
+            const position = Position.create(0, 5)
+            const result = ExpressionUtils.getExpressionAtPosition(doc, position)
+            assert.notStrictEqual(result, null)
+            assert.strictEqual(result?.fullExpression, 'someVariable')
+            assert.strictEqual(result?.targetExpression, 'someVariable')
+            assert.strictEqual(result?.unqualifiedTarget, 'someVariable')
+            assert.strictEqual(result?.first, 'someVariable')
+            assert.strictEqual(result?.last, 'someVariable')
+        })
+
+        it('should return correct expression for dotted identifier', () => {
+            const doc = TextDocument.create('', '', 1, 'pkg.Class.method')
+            const position = Position.create(0, 7)
+            const result = ExpressionUtils.getExpressionAtPosition(doc, position)
+            assert.notStrictEqual(result, null)
+            assert.strictEqual(result?.fullExpression, 'pkg.Class.method')
+            assert.strictEqual(result?.targetExpression, 'pkg.Class')
+            assert.strictEqual(result?.unqualifiedTarget, 'Class')
+            assert.strictEqual(result?.first, 'pkg')
+            assert.strictEqual(result?.last, 'method')
+        })
+
+        it('should handle position at the end of identifier', () => {
+            const doc = TextDocument.create('', '', 1, 'someVariable')
+            const position = Position.create(0, 12)
+            const result = ExpressionUtils.getExpressionAtPosition(doc, position)
+            assert.notStrictEqual(result, null)
+            assert.strictEqual(result?.fullExpression, 'someVariable')
+        })
+
+        it('should handle multiple identifiers on the same line', () => {
+            const doc = TextDocument.create('', '', 1, 'firstVar secondVar thirdVar')
+            const position = Position.create(0, 15)
+            const result = ExpressionUtils.getExpressionAtPosition(doc, position)
+            assert.notStrictEqual(result, null)
+            assert.strictEqual(result?.fullExpression, 'secondVar')
+        })
+
+        it('should return null for position between identifiers', () => {
+            const doc = TextDocument.create('', '', 1, 'firstVar    secondVar')
+            const position = Position.create(0, 10)
+            const result = ExpressionUtils.getExpressionAtPosition(doc, position)
+            assert.strictEqual(result, null)
+        })
+
+        it('should match identifier next to dot operator', () => {
+            const doc = TextDocument.create('', '', 1, 'var.^2')
+            const position = Position.create(0, 2)
+            const result = ExpressionUtils.getExpressionAtPosition(doc, position)
+            assert.notStrictEqual(result, null)
+            assert.strictEqual(result?.fullExpression, 'var')
+        })
+
+        it('should return null for a non-identifier character', () => {
+            const doc = TextDocument.create('', '', 1, 'var.@')
+            const position = Position.create(0, 4)
+            const result = ExpressionUtils.getExpressionAtPosition(doc, position)
+            assert.strictEqual(result, null)
+        })
+
+        it('should return null for a number', () => {
+            const doc = TextDocument.create('', '', 1, '42')
+            const position = Position.create(0, 1)
+            const result = ExpressionUtils.getExpressionAtPosition(doc, position)
+            assert.strictEqual(result, null)
+        })
+    })
+})

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`/// <reference types="node" />`
	`2`	`+/// <reference types="node" />`
`2`	`3`	`import { ChildProcess } from 'child_process';`
`3`	`4`	`declare const Faye: any;`
`4`	`5`	`declare type Client = typeof Faye.Client;`