Show warning and allow user to turn off smart send for deprecated Pyt…

…hon code (microsoft#22353) Resolves: microsoft#22341 microsoft#22340 Showing warning message after detecting user is on Python file with deprecated Python code, and are attempting to run smart send via shift+enter action. Allow user to turn off this via workspace setting. --------- Co-authored-by: Courtney Webster <60238438+cwebster-99@users.noreply.github.com> Co-authored-by: Kartik Raj <karraj@microsoft.com>
anthonykim1 · Nov 18, 2023 · 3c552f9 · 3c552f9
1 parent ef983f4
commit 3c552f9
Show file tree

Hide file tree

Showing 14 changed files with 184 additions and 17 deletions.
diff --git a/package.json b/package.json
@@ -704,6 +704,12 @@
                     "scope": "resource",
                     "type": "array"
                 },
+                "python.REPL.enableREPLSmartSend": {
+                    "default": true,
+                    "description": "%python.EnableREPLSmartSend.description%",
+                    "scope": "resource",
+                    "type": "boolean"
+                },
                 "python.testing.autoTestDiscoverOnSaveEnabled": {
                     "default": true,
                     "description": "%python.testing.autoTestDiscoverOnSaveEnabled.description%",

diff --git a/package.nls.json b/package.nls.json
@@ -59,6 +59,7 @@
     "python.missingPackage.severity.description": "Set severity of missing packages in requirements.txt or pyproject.toml",
     "python.pipenvPath.description": "Path to the pipenv executable to use for activation.",
     "python.poetryPath.description": "Path to the poetry executable.",
+    "python.EnableREPLSmartSend.description": "Toggle Smart Send for the Python REPL. Smart Send enables sending the smallest runnable block of code to the REPL on Shift+Enter and moves the cursor accordingly.",
     "python.tensorBoard.logDirectory.description": "Set this setting to your preferred TensorBoard log directory to skip log directory prompt when starting TensorBoard.",
     "python.tensorBoard.logDirectory.markdownDeprecationMessage": "Tensorboard support has been moved to the extension [Tensorboard extension](https://marketplace.visualstudio.com/items?itemName=ms-toolsai.tensorboard). Instead use the setting `tensorBoard.logDirectory`.",
     "python.tensorBoard.logDirectory.deprecationMessage": "Tensorboard support has been moved to the extension Tensorboard extension. Instead use the setting `tensorBoard.logDirectory`.",

diff --git a/pythonFiles/normalizeSelection.py b/pythonFiles/normalizeSelection.py
@@ -150,8 +150,18 @@ def traverse_file(wholeFileContent, start_line, end_line, was_highlighted):
     or a multiline dictionary, or differently styled multi-line list comprehension, etc.
     Then call the normalize_lines function to normalize our smartly selected code block.
     """
+    parsed_file_content = None
+
+    try:
+        parsed_file_content = ast.parse(wholeFileContent)
+    except Exception:
+        # Handle case where user is attempting to run code where file contains deprecated Python code.
+        # Let typescript side know and show warning message.
+        return {
+            "normalized_smart_result": "deprecated",
+            "which_line_next": 0,
+        }
 
-    parsed_file_content = ast.parse(wholeFileContent)
     smart_code = ""
     should_run_top_blocks = []
 
@@ -267,7 +277,11 @@ def get_next_block_lineno(which_line_next):
     data = None
     which_line_next = 0
 
-    if empty_Highlight and contents.get("smartSendExperimentEnabled"):
+    if (
+        empty_Highlight
+        and contents.get("smartSendExperimentEnabled")
+        and contents.get("smartSendSettingsEnabled")
+    ):
         result = traverse_file(
             contents["wholeFileContent"],
             vscode_start_line,
@@ -276,9 +290,12 @@ def get_next_block_lineno(which_line_next):
         )
         normalized = result["normalized_smart_result"]
         which_line_next = result["which_line_next"]
-        data = json.dumps(
-            {"normalized": normalized, "nextBlockLineno": result["which_line_next"]}
-        )
+        if normalized == "deprecated":
+            data = json.dumps({"normalized": normalized})
+        else:
+            data = json.dumps(
+                {"normalized": normalized, "nextBlockLineno": result["which_line_next"]}
+            )
     else:
         normalized = normalize_lines(contents["code"])
         data = json.dumps({"normalized": normalized})

diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts
@@ -72,6 +72,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
     ];
     ['workbench.action.files.openFolder']: [];
     ['workbench.action.openWorkspace']: [];
+    ['workbench.action.openSettings']: [string];
     ['setContext']: [string, boolean] | ['python.vscode.channel', Channel];
     ['python.reloadVSCode']: [string];
     ['revealLine']: [{ lineNumber: number; at: 'top' | 'center' | 'bottom' }];

diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts
@@ -28,6 +28,7 @@ import {
     IInterpreterPathService,
     IInterpreterSettings,
     IPythonSettings,
+    IREPLSettings,
     ITensorBoardSettings,
     ITerminalSettings,
     Resource,
@@ -111,6 +112,8 @@ export class PythonSettings implements IPythonSettings {
 
     public globalModuleInstallation = false;
 
+    public REPL!: IREPLSettings;
+
     public experiments!: IExperiments;
 
     public languageServer: LanguageServerType = LanguageServerType.Node;
@@ -360,7 +363,8 @@ export class PythonSettings implements IPythonSettings {
                   activateEnvInCurrentTerminal: false,
               };
 
-        const experiments = systemVariables.resolveAny(pythonSettings.get<IExperiments>('experiments'))!;
+        this.REPL = pythonSettings.get<IREPLSettings>('REPL')!;
+        const experiments = pythonSettings.get<IExperiments>('experiments')!;
         if (this.experiments) {
             Object.assign<IExperiments, IExperiments>(this.experiments, experiments);
         } else {

diff --git a/src/client/common/types.ts b/src/client/common/types.ts
@@ -178,6 +178,7 @@ export interface IPythonSettings {
     readonly languageServerIsDefault: boolean;
     readonly defaultInterpreterPath: string;
     readonly tensorBoard: ITensorBoardSettings | undefined;
+    readonly REPL: IREPLSettings;
     register(): void;
 }
 
@@ -197,6 +198,10 @@ export interface ITerminalSettings {
     readonly activateEnvInCurrentTerminal: boolean;
 }
 
+export interface IREPLSettings {
+    readonly enableREPLSmartSend: boolean;
+}
+
 export interface IExperiments {
     /**
      * Return `true` if experiments are enabled, else `false`.

diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts
@@ -4,6 +4,7 @@
 'use strict';
 
 import { l10n } from 'vscode';
+import { Commands } from '../constants';
 
 /* eslint-disable @typescript-eslint/no-namespace, no-shadow */
 
@@ -42,6 +43,9 @@ export namespace Diagnostics {
     export const pylanceDefaultMessage = l10n.t(
         "The Python extension now includes Pylance to improve completions, code navigation, overall performance and much more! You can learn more about the update and learn how to change your language server [here](https://aka.ms/new-python-bundle).\n\nRead Pylance's license [here](https://marketplace.visualstudio.com/items/ms-python.vscode-pylance/license).",
     );
+    export const invalidSmartSendMessage = l10n.t(`Python is unable to parse the code provided. Please
+    turn off Smart Send if you wish to always run line by line or explicitly select code
+    to force run. See [logs](command:${Commands.ViewOutput}) for more details`);
 }
 
 export namespace Common {
@@ -92,6 +96,9 @@ export namespace AttachProcess {
     export const refreshList = l10n.t('Refresh process list');
 }
 
+export namespace Repl {
+    export const disableSmartSend = l10n.t('Disable Smart Send');
+}
 export namespace Pylance {
     export const remindMeLater = l10n.t('Remind me later');
 

diff --git a/src/client/terminals/codeExecution/djangoShellCodeExecution.ts b/src/client/terminals/codeExecution/djangoShellCodeExecution.ts
@@ -36,6 +36,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
             disposableRegistry,
             platformService,
             interpreterService,
+            commandManager,
         );
         this.terminalTitle = 'Django Shell';
         disposableRegistry.push(new DjangoContextInitializer(documentManager, workspace, fileSystem, commandManager));

diff --git a/src/client/terminals/codeExecution/helper.ts b/src/client/terminals/codeExecution/helper.ts
@@ -6,6 +6,7 @@ import { inject, injectable } from 'inversify';
 import { l10n, Position, Range, TextEditor, Uri } from 'vscode';
 
 import {
+    IActiveResourceService,
     IApplicationShell,
     ICommandManager,
     IDocumentManager,
@@ -36,6 +37,8 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
 
     private readonly commandManager: ICommandManager;
 
+    private activeResourceService: IActiveResourceService;
+
     // eslint-disable-next-line @typescript-eslint/ban-ts-comment
     // @ts-expect-error TS6133: 'configSettings' is declared but its value is never read.
     private readonly configSettings: IConfigurationService;
@@ -47,6 +50,7 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
         this.interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
         this.configSettings = serviceContainer.get<IConfigurationService>(IConfigurationService);
         this.commandManager = serviceContainer.get<ICommandManager>(ICommandManager);
+        this.activeResourceService = this.serviceContainer.get<IActiveResourceService>(IActiveResourceService);
     }
 
     public async normalizeLines(code: string, wholeFileContent?: string, resource?: Uri): Promise<string> {
@@ -90,13 +94,21 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
             const endLineVal = activeEditor?.selection?.end.line ?? 0;
             const emptyHighlightVal = activeEditor?.selection?.isEmpty ?? true;
             const smartSendExperimentEnabledVal = pythonSmartSendEnabled(this.serviceContainer);
+            let smartSendSettingsEnabledVal = false;
+            const configuration = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
+            if (configuration) {
+                const pythonSettings = configuration.getSettings(this.activeResourceService.getActiveResource());
+                smartSendSettingsEnabledVal = pythonSettings.REPL.enableREPLSmartSend;
+            }
+
             const input = JSON.stringify({
                 code,
                 wholeFileContent,
                 startLine: startLineVal,
                 endLine: endLineVal,
                 emptyHighlight: emptyHighlightVal,
                 smartSendExperimentEnabled: smartSendExperimentEnabledVal,
+                smartSendSettingsEnabled: smartSendSettingsEnabledVal,
             });
             observable.proc?.stdin?.write(input);
             observable.proc?.stdin?.end();
@@ -105,7 +117,12 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
             const result = await normalizeOutput.promise;
             const object = JSON.parse(result);
 
-            if (activeEditor?.selection) {
+            if (
+                activeEditor?.selection &&
+                smartSendExperimentEnabledVal &&
+                smartSendSettingsEnabledVal &&
+                object.normalized !== 'deprecated'
+            ) {
                 const lineOffset = object.nextBlockLineno - activeEditor!.selection.start.line - 1;
                 await this.moveToNextBlock(lineOffset, activeEditor);
             }

diff --git a/src/client/terminals/codeExecution/repl.ts b/src/client/terminals/codeExecution/repl.ts
@@ -5,7 +5,7 @@
 
 import { inject, injectable } from 'inversify';
 import { Disposable } from 'vscode';
-import { IWorkspaceService } from '../../common/application/types';
+import { ICommandManager, IWorkspaceService } from '../../common/application/types';
 import { IPlatformService } from '../../common/platform/types';
 import { ITerminalServiceFactory } from '../../common/terminal/types';
 import { IConfigurationService, IDisposableRegistry } from '../../common/types';
@@ -21,6 +21,7 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
         @inject(IDisposableRegistry) disposableRegistry: Disposable[],
         @inject(IPlatformService) platformService: IPlatformService,
         @inject(IInterpreterService) interpreterService: IInterpreterService,
+        @inject(ICommandManager) commandManager: ICommandManager,
     ) {
         super(
             terminalServiceFactory,
@@ -29,6 +30,7 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
             disposableRegistry,
             platformService,
             interpreterService,
+            commandManager,
         );
         this.terminalTitle = 'REPL';
     }

diff --git a/src/client/terminals/codeExecution/terminalCodeExecution.ts b/src/client/terminals/codeExecution/terminalCodeExecution.ts
@@ -6,15 +6,17 @@
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { Disposable, Uri } from 'vscode';
-import { IWorkspaceService } from '../../common/application/types';
+import { ICommandManager, IWorkspaceService } from '../../common/application/types';
 import '../../common/extensions';
 import { IPlatformService } from '../../common/platform/types';
 import { ITerminalService, ITerminalServiceFactory } from '../../common/terminal/types';
 import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types';
+import { Diagnostics, Repl } from '../../common/utils/localize';
+import { showWarningMessage } from '../../common/vscodeApis/windowApis';
 import { IInterpreterService } from '../../interpreter/contracts';
+import { traceInfo } from '../../logging';
 import { buildPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/exec';
 import { ICodeExecutionService } from '../../terminals/types';
-
 @injectable()
 export class TerminalCodeExecutionProvider implements ICodeExecutionService {
     private hasRanOutsideCurrentDrive = false;
@@ -27,6 +29,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
         @inject(IDisposableRegistry) protected readonly disposables: Disposable[],
         @inject(IPlatformService) protected readonly platformService: IPlatformService,
         @inject(IInterpreterService) protected readonly interpreterService: IInterpreterService,
+        @inject(ICommandManager) protected readonly commandManager: ICommandManager,
     ) {}
 
     public async executeFile(file: Uri, options?: { newTerminalPerFile: boolean }) {
@@ -42,9 +45,17 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
         if (!code || code.trim().length === 0) {
             return;
         }
-
         await this.initializeRepl(resource);
-        await this.getTerminalService(resource).sendText(code);
+        if (code == 'deprecated') {
+            // If user is trying to smart send deprecated code show warning
+            const selection = await showWarningMessage(Diagnostics.invalidSmartSendMessage, Repl.disableSmartSend);
+            traceInfo(`Selected file contains invalid Python or Deprecated Python 2 code`);
+            if (selection === Repl.disableSmartSend) {
+                this.configurationService.updateSetting('REPL.enableREPLSmartSend', false, resource);
+            }
+        } else {
+            await this.getTerminalService(resource).sendText(code);
+        }
     }
     public async initializeRepl(resource: Resource) {
         const terminalService = this.getTerminalService(resource);

diff --git a/src/test/pythonFiles/terminalExec/sample_invalid_smart_selection.py b/src/test/pythonFiles/terminalExec/sample_invalid_smart_selection.py
@@ -0,0 +1,10 @@
+def beliebig(x, y, *mehr):
+    print "x=", x, ", x=", y
+    print "mehr: ", mehr
+
+list = [
+1,
+2,
+3,
+]
+print("Above is invalid");print("deprecated");print("show warning")