Remove extraneous CR characters from normalized code snippets. (microsoft#2927)

d3r3kk · web-flow · commit d7a8f3a62879 · 2018-10-17T14:46:13.000-07:00
Remove extraneous CR characters from normalized code snippets.

- Correct normalization python script to use LF instead of CRLF to split lines.
  - On windows, Python will inject CR whenever it sees an LF written to stdout.
  - This was the root cause of the CRCRLF coming back from normalization.
- Change helper.unit.test to helper.test, it shells out to Python and is not a unit test suite.
- Re-enable tests for helper.
diff --git a/news/2 Fixes/2857.md b/news/2 Fixes/2857.md
@@ -0,0 +1 @@
+Stop `normalizationForInterpreter.py` script from returning CRCRLF line-endings.
diff --git a/pythonFiles/normalizeForInterpreter.py b/pythonFiles/normalizeForInterpreter.py
@@ -81,10 +81,10 @@ def normalize_lines(source):
     # we have indented code.
     if (len(lines) > 1 and len(''.join(lines[-2:])) == 0) \
         or source.endswith(('\n\n', '\r\n\r\n')):
-        trailing_newline = os.linesep * 2
+        trailing_newline = '\n' * 2
     # Find out if we have any trailing blank lines
     elif len(lines[-1].strip()) == 0 or source.endswith(('\n', '\r\n')):
-        trailing_newline = os.linesep
+        trailing_newline = '\n'
     else:
         trailing_newline = ''
 
@@ -101,7 +101,7 @@ def normalize_lines(source):
     # Step 2: Add blank lines between each global statement block.
     # A consequtive single lines blocks of code will be treated as a single statement,
     # just to ensure we do not unnecessarily add too many blank lines.
-    source = os.linesep.join(lines)
+    source = '\n'.join(lines)
     tokens = _tokenize(source)
     dedent_indexes = (spos[0] for (toknum, tokval, spos, epos, line) in tokens
                                 if toknum == token.DEDENT and _indent_size(line) == 0)
@@ -111,7 +111,7 @@ def normalize_lines(source):
     for line_number in filter(lambda x: x > 1, start_positions):
         lines.insert(line_number-1, '')
 
-    sys.stdout.write(os.linesep.join(lines) + trailing_newline)
+    sys.stdout.write('\n'.join(lines) + trailing_newline)
     sys.stdout.flush()
 
 
diff --git a/src/client/terminals/codeExecution/helper.ts b/src/client/terminals/codeExecution/helper.ts
@@ -33,6 +33,7 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
             const args = [path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'normalizeForInterpreter.py'), code];
             const processService = await this.processServiceFactory.create(resource);
             const proc = await processService.exec(pythonPath, args, { throwOnStdErr: true });
+
             return proc.stdout;
         } catch (ex) {
             console.error(ex, 'Python: Failed to normalize code for execution in terminal');
diff --git a/src/test/terminals/codeExecution/helper.test.ts b/src/test/terminals/codeExecution/helper.test.ts
@@ -32,7 +32,7 @@ suite('Terminal - Code Execution Helper', () => {
     let editor: TypeMoq.IMock<TextEditor>;
     let processService: TypeMoq.IMock<IProcessService>;
     let configService: TypeMoq.IMock<IConfigurationService>;
-    setup(function () {
+    setup(() => {
         const serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
         documentManager = TypeMoq.Mock.ofType<IDocumentManager>();
         applicationShell = TypeMoq.Mock.ofType<IApplicationShell>();
@@ -58,7 +58,7 @@ suite('Terminal - Code Execution Helper', () => {
         editor.setup(e => e.document).returns(() => document.object);
 
         // tslint:disable-next-line:no-invalid-this
-        this.skip();
+        // this.skip();
     });
 
     async function ensureBlankLinesAreRemoved(source: string, expectedSource: string) {
@@ -77,6 +77,17 @@ suite('Terminal - Code Execution Helper', () => {
         const expectedCode = code.filter(line => line.trim().length > 0).join(EOL);
         await ensureBlankLinesAreRemoved(code.join(EOL), expectedCode);
     });
+    test('Ensure there are no multiple-CR elements in the normalized code.', async () => {
+        const code = ['import sys', '', '', '', 'print(sys.executable)', '', 'print("1234")', '', '', 'print(1)', 'print(2)'];
+        const actualProcessService = new ProcessService(new BufferDecoder());
+        processService.setup(p => p.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
+            .returns((file, args, options) => {
+                return actualProcessService.exec.apply(actualProcessService, [file, args, options]);
+            });
+        const normalizedCode = await helper.normalizeLines(code.join(EOL));
+        const doubleCrIndex = normalizedCode.indexOf('\r\r');
+        expect(doubleCrIndex).to.be.equal(-1, 'Double CR (CRCRLF) line endings detected in normalized code snippet.');
+    });
     ['', '1', '2', '3', '4', '5', '6', '7'].forEach(fileNameSuffix => {
         test(`Ensure blank lines are removed (Sample${fileNameSuffix})`, async () => {
             const code = await fs.readFile(path.join(TEST_FILES_PATH, `sample${fileNameSuffix}_raw.py`), 'utf8');

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Stop `normalizationForInterpreter.py` script from returning CRCRLF line-endings.