support library code in fourslash test and added missing typestub dia… (

microsoft#88) * support library code in fourslash test and added missing typestub diagnostic test * Squashed 'server/pyright/' changes from 9521693..b1e0ff8 b1e0ff8 made reportMissingTypeStub diagnostics warning by default and some refactoring around code actions. (microsoft#507) git-subtree-dir: server/pyright git-subtree-split: b1e0ff8 * addressed PR feedback and fix python syntax error on python test code * Squashed 'server/pyright/' changes from 9521693..b1e0ff8 b1e0ff8 made reportMissingTypeStub diagnostics warning by default and some refactoring around code actions. (microsoft#507) git-subtree-dir: server/pyright git-subtree-split: b1e0ff8 * added site-packages to consts.ts as well
heejaechang · Feb 13, 2020 · 4b97037 · 4b97037
1 parent 26cf0a6
commit 4b97037
Show file tree

Hide file tree

Showing 13 changed files with 118 additions and 49 deletions.
diff --git a/server/src/analyzer/importResolver.ts b/server/src/analyzer/importResolver.ts
@@ -16,10 +16,10 @@ import {
 } from '../common/pathUtils';
 import { versionToString } from '../common/pythonVersion';
 import * as StringUtils from '../common/stringUtils';
+import { VirtualFileSystem } from '../common/vfs';
 import { ImplicitImport, ImportResult, ImportType } from './importResult';
 import * as PythonPathUtils from './pythonPathUtils';
 import { isDunderName } from './symbolNameUtils';
-import { VirtualFileSystem } from '../common/vfs';
 
 export interface ImportedModuleDescriptor {
     leadingDots: number;

diff --git a/server/src/analyzer/program.ts b/server/src/analyzer/program.ts
@@ -7,7 +7,6 @@
 * An object that tracks all of the source files being analyzed
 * and all of their recursive imports.
 */
-
 import * as assert from 'assert';
 import { CompletionItem, CompletionList, DocumentSymbol, SymbolInformation } from 'vscode-languageserver';
 
@@ -20,6 +19,7 @@ import {
     combinePaths, getDirectoryPath, getRelativePath, makeDirectories,
     normalizePath, stripFileExtension
 } from '../common/pathUtils';
+import { DocumentRange, doRangesOverlap, Position, Range } from '../common/textRange';
 import { Duration, timingStats } from '../common/timing';
 import { ModuleSymbolMap } from '../languageService/completionProvider';
 import { HoverResults } from '../languageService/hoverProvider';
@@ -34,7 +34,6 @@ import { SourceFile } from './sourceFile';
 import { SymbolTable } from './symbol';
 import { createTypeEvaluator, TypeEvaluator } from './typeEvaluator';
 import { TypeStubWriter } from './typeStubWriter';
-import { Position, Range, DocumentRange, doRangesOverlap } from '../common/textRange';
 
 const _maxImportDepth = 256;
 

diff --git a/server/src/analyzer/pythonPathUtils.ts b/server/src/analyzer/pythonPathUtils.ts
@@ -50,22 +50,22 @@ export function findPythonSearchPaths(fs: VirtualFileSystem, configOptions: Conf
     }
 
     if (venvPath) {
-        let libPath = combinePaths(venvPath, 'lib');
+        let libPath = combinePaths(venvPath, consts.LIB);
         if (fs.existsSync(libPath)) {
-            importFailureInfo.push(`Found path '${ libPath }'; looking for site-packages`);
+            importFailureInfo.push(`Found path '${ libPath }'; looking for ${ consts.SITE_PACKAGES }`);
         } else {
             importFailureInfo.push(`Did not find '${ libPath }'; trying 'Lib' instead`);
             libPath = combinePaths(venvPath, 'Lib');
             if (fs.existsSync(libPath)) {
-                importFailureInfo.push(`Found path '${ libPath }'; looking for site-packages`);
+                importFailureInfo.push(`Found path '${ libPath }'; looking for ${ consts.SITE_PACKAGES }`);
             } else {
                 importFailureInfo.push(`Did not find '${ libPath }'`);
                 libPath = '';
             }
         }
 
         if (libPath) {
-            const sitePackagesPath = combinePaths(libPath, 'site-packages');
+            const sitePackagesPath = combinePaths(libPath, consts.SITE_PACKAGES);
             if (fs.existsSync(sitePackagesPath)) {
                 importFailureInfo.push(`Found path '${ sitePackagesPath }'`);
                 return [sitePackagesPath];
@@ -79,7 +79,7 @@ export function findPythonSearchPaths(fs: VirtualFileSystem, configOptions: Conf
             for (let i = 0; i < entries.directories.length; i++) {
                 const dirName = entries.directories[i];
                 if (dirName.startsWith('python')) {
-                    const dirPath = combinePaths(libPath, dirName, 'site-packages');
+                    const dirPath = combinePaths(libPath, dirName, consts.SITE_PACKAGES);
                     if (fs.existsSync(dirPath)) {
                         importFailureInfo.push(`Found path '${ dirPath }'`);
                         return [dirPath];
@@ -90,7 +90,7 @@ export function findPythonSearchPaths(fs: VirtualFileSystem, configOptions: Conf
             }
         }
 
-        importFailureInfo.push(`Did not find site-packages. Falling back on python interpreter.`);
+        importFailureInfo.push(`Did not find '${ consts.SITE_PACKAGES }'. Falling back on python interpreter.`);
     }
 
     // Fall back on the python interpreter.

diff --git a/server/src/common/consts.ts b/server/src/common/consts.ts
@@ -7,3 +7,5 @@
  */
 
 export const TYPESHED_FALLBACK = 'typeshed-fallback';
+export const LIB = 'lib';
+export const SITE_PACKAGES = 'site-packages';
diff --git a/server/src/common/core.ts b/server/src/common/core.ts
@@ -95,3 +95,16 @@ export interface MapLike<T> {
 export function hasProperty(map: MapLike<any>, key: string): boolean {
     return hasOwnProperty.call(map, key);
 }
+
+/**
+ * Convert the given value to boolean
+ * @param trueOrFalse string value 'true' or 'false'
+ */
+export function toBoolean(trueOrFalse: string): boolean {
+    const normalized = trueOrFalse?.trim().toUpperCase();
+    if (normalized === 'TRUE') {
+        return true;
+    }
+
+    return false;
+}
diff --git a/server/src/tests/fourSlashParser.test.ts b/server/src/tests/fourSlashParser.test.ts
@@ -8,7 +8,7 @@
  */
 
 import * as assert from 'assert';
-import { getBaseFileName, normalizeSlashes } from '../common/pathUtils';
+import { combinePaths, getBaseFileName, normalizeSlashes } from '../common/pathUtils';
 import { compareStringsCaseSensitive } from '../common/stringUtils';
 import { parseTestData } from './harness/fourslash/fourSlashParser';
 import { CompilerSettings } from './harness/fourslash/fourSlashTypes';
@@ -55,18 +55,34 @@ test('Filename', () => {
 });
 
 test('Extra file options', () => {
-    // filename must be last file options
+    // filename must be the first file options
     const code = `
-// @reserved: not used
 // @filename: file1.py
+// @library: false
 ////class A:
 ////    pass
     `;
 
     const data = parseTestData('.', code, 'test.py');
+
+    assert.equal(data.files[0].fileName, normalizeSlashes('./file1.py'));
+
     assertOptions(data.globalOptions, []);
+    assertOptions(data.files[0].fileOptions, [['filename', 'file1.py'], ['library', 'false']]);
+});
 
-    assertOptions(data.files[0].fileOptions, [['filename', 'file1.py'], ['reserved', 'not used']]);
+test('Library options', () => {
+    // filename must be the first file options
+    const code = `
+// @filename: file1.py
+// @library: true
+////class A:
+////    pass
+    `;
+
+    const data = parseTestData('.', code, 'test.py');
+
+    assert.equal(data.files[0].fileName, normalizeSlashes(combinePaths(factory.libFolder, 'file1.py')));
 });
 
 test('Range', () => {
@@ -180,10 +196,12 @@ test('Multiple Files', () => {
     // range can have 1 marker in it
     const code = `
 // @filename: src/A.py
+// @library: false
 ////class A:
 ////    pass
 
 // @filename: src/B.py
+// @library: true
 ////class B:
 ////    pass
 
@@ -196,7 +214,8 @@ test('Multiple Files', () => {
     assert.equal(data.files.length, 3);
 
     assert.equal(data.files.filter(f => f.fileName === normalizeSlashes('./src/A.py'))[0].content, getContent('A'));
-    assert.equal(data.files.filter(f => f.fileName === normalizeSlashes('./src/B.py'))[0].content, getContent('B'));
+    assert.equal(data.files.filter(f => f.fileName ===
+        normalizeSlashes(combinePaths(factory.libFolder, 'src/B.py')))[0].content, getContent('B'));
     assert.equal(data.files.filter(f => f.fileName === normalizeSlashes('./src/C.py'))[0].content, getContent('C'));
 });
 

diff --git a/server/src/tests/fourslash/missingTypeStub.fourslash.ts b/server/src/tests/fourslash/missingTypeStub.fourslash.ts
@@ -0,0 +1,20 @@
+/// <reference path="fourslash.ts" />
+
+// @filename: mspythonconfig.json
+//// {
+////   "reportMissingTypeStubs": "warning"
+//// }
+
+// @filename: testLib/__init__.py
+// @library: true
+//// # This is a library file
+//// class MyLibrary:
+////     def DoEveryThing(self, code: str):
+////         pass
+
+// @filename: test.py
+//// import [|/*marker*/testLi|]b
+
+helper.verifyDiagnostics({
+    'marker': { category: 'warning', message: 'Stub file not found for \'testLib\'' }
+});
diff --git a/server/src/tests/harness/fourslash/fourSlashParser.ts b/server/src/tests/harness/fourslash/fourSlashParser.ts
@@ -7,7 +7,9 @@
  */
 
 import { contains } from '../../../common/collectionUtils';
-import { combinePaths, isRootedDiskPath, normalizeSlashes } from '../../../common/pathUtils';
+import { toBoolean } from '../../../common/core';
+import { combinePaths, getRelativePath, isRootedDiskPath, normalizePath, normalizeSlashes } from '../../../common/pathUtils';
+import { libFolder } from '../vfs/factory';
 import { fileMetadataNames, FourSlashData, FourSlashFile, Marker, MetadataOptionNames, Range } from './fourSlashTypes';
 
 /**
@@ -48,6 +50,10 @@ export function parseTestData(basePath: string, contents: string, fileName: stri
     function nextFile() {
         if (currentFileContent === undefined) { return; }
 
+        if (toBoolean(currentFileOptions[MetadataOptionNames.library])) {
+            currentFileName = normalizePath(combinePaths(libFolder, getRelativePath(currentFileName, normalizedBasePath)));
+        }
+
         const file = parseFileContent(currentFileContent, currentFileName, markerPositions, markers, ranges);
         file.fileOptions = currentFileOptions;
 
@@ -85,14 +91,14 @@ export function parseTestData(basePath: string, contents: string, fileName: stri
                 } else {
                     switch (key) {
                         case MetadataOptionNames.fileName: {
-                                // Found an @FileName directive, if this is not the first then create a new subfile
-                                nextFile();
-                                const normalizedPath = normalizeSlashes(value);
-                                currentFileName = isRootedDiskPath(normalizedPath) ? normalizedPath :
-                                    combinePaths(normalizedBasePath, normalizedPath);
-                                currentFileOptions[key] = value;
-                                break;
-                            }
+                            // Found an @FileName directive, if this is not the first then create a new subfile
+                            nextFile();
+                            const normalizedPath = normalizeSlashes(value);
+                            currentFileName = isRootedDiskPath(normalizedPath) ? normalizedPath :
+                                combinePaths(normalizedBasePath, normalizedPath);
+                            currentFileOptions[key] = value;
+                            break;
+                        }
                         default:
                             // Add other fileMetadata flag
                             currentFileOptions[key] = value;

diff --git a/server/src/tests/harness/fourslash/fourSlashTypes.ts b/server/src/tests/harness/fourslash/fourSlashTypes.ts
@@ -8,7 +8,7 @@
 import * as debug from '../../../common/debug';
 
 /** setting file name */
-export const pythonSettingFilename = 'python.json';
+export const pythonSettingFilename = 'mspythonconfig.json';
 
 /** well known global option names */
 export const enum GlobalMetadataOptionNames {
@@ -20,11 +20,11 @@ export const enum GlobalMetadataOptionNames {
 /** Any option name not belong to this will become global option */
 export const enum MetadataOptionNames {
     fileName = 'filename',
-    reserved = 'reserved'
+    library = 'library'
 }
 
 /** List of allowed file metadata names */
-export const fileMetadataNames = [MetadataOptionNames.fileName, MetadataOptionNames.reserved];
+export const fileMetadataNames = [MetadataOptionNames.fileName, MetadataOptionNames.library];
 
 /** all the necessary information to set the right compiler settings */
 export interface CompilerSettings {

diff --git a/server/src/tests/harness/fourslash/testState.ts b/server/src/tests/harness/fourslash/testState.ts
@@ -15,7 +15,7 @@ import { Program } from '../../../analyzer/program';
 import { AnalyzerService } from '../../../analyzer/service';
 import { ConfigOptions } from '../../../common/configOptions';
 import { NullConsole } from '../../../common/console';
-import { Comparison, isNumber, isString } from '../../../common/core';
+import { Comparison, isNumber, isString, toBoolean } from '../../../common/core';
 import * as debug from '../../../common/debug';
 import { DiagnosticCategory } from '../../../common/diagnostic';
 import { combinePaths, comparePaths, getBaseFileName, normalizePath, normalizeSlashes } from '../../../common/pathUtils';
@@ -28,7 +28,7 @@ import { createFromFileSystem } from '../vfs/factory';
 import * as vfs from '../vfs/filesystem';
 import {
     CompilerSettings, FourSlashData, FourSlashFile, GlobalMetadataOptionNames, Marker,
-    MultiMap, pythonSettingFilename, Range, TestCancellationToken
+    MetadataOptionNames, MultiMap, pythonSettingFilename, Range, TestCancellationToken
 } from './fourSlashTypes';
 
 export interface TextChange {
@@ -64,6 +64,7 @@ export class TestState {
         this._cancellationToken = new TestCancellationToken();
         const configOptions = this._convertGlobalOptionsToConfigOptions(this.testData.globalOptions);
 
+        const sourceFiles = [];
         const files: vfs.FileSet = {};
         for (const file of testData.files) {
             // if one of file is configuration file, set config options from the given json
@@ -76,8 +77,13 @@ export class TestState {
                 }
 
                 configOptions.initializeFromJson(configJson, new NullConsole());
+                this._applyTestConfigOptions(configOptions);
             } else {
                 files[file.fileName] = new vfs.File(file.content, { meta: file.fileOptions, encoding: 'utf8' });
+
+                if (!toBoolean(file.fileOptions[MetadataOptionNames.library])) {
+                    sourceFiles.push(file.fileName);
+                }
             }
         }
 
@@ -88,7 +94,7 @@ export class TestState {
         // this should be change to AnalyzerService rather than Program
         const importResolver = importResolverFactory(fs, configOptions);
         const program = new Program(importResolver, configOptions);
-        program.setTrackedFiles(Object.keys(files));
+        program.setTrackedFiles(sourceFiles);
 
         // make sure these states are consistent between these objects.
         // later make sure we just hold onto AnalyzerService and get all these
@@ -97,7 +103,7 @@ export class TestState {
         this.configOptions = configOptions;
         this.importResolver = importResolver;
         this.program = program;
-        this._files.push(...Object.keys(files));
+        this._files = sourceFiles;
 
         if (this._files.length > 0) {
             // Open the first file by default
@@ -368,7 +374,7 @@ export class TestState {
 
         // expected number of files
         if (resultPerFile.size !== rangePerFile.size) {
-            this._raiseError(`actual and expected doesn't match - expected: ${ stringify(rangePerFile) }, actual: ${ stringify(rangePerFile) }`);
+            this._raiseError(`actual and expected doesn't match - expected: ${ stringify(resultPerFile) }, actual: ${ stringify(rangePerFile) }`);
         }
 
         for (const [file, ranges] of rangePerFile.entries()) {
@@ -469,8 +475,18 @@ export class TestState {
 
         // add more global options as we need them
 
+        return this._applyTestConfigOptions(configOptions);
+    }
+
+    private _applyTestConfigOptions(configOptions: ConfigOptions) {
         // Always enable "test mode".
         configOptions.internalTestMode = true;
+
+        // run test in venv mode under root so that
+        // under test we can point to local lib folder
+        configOptions.venvPath = vfs.MODULE_PATH;
+        configOptions.defaultVenv = vfs.MODULE_PATH;
+
         return configOptions;
     }
 

diff --git a/server/src/tests/harness/vfs/factory.ts b/server/src/tests/harness/vfs/factory.ts
@@ -30,6 +30,7 @@ export interface FileSystemCreateOptions extends FileSystemOptions {
     documents?: readonly TextDocument[];
 }
 
+export const libFolder = combinePaths(MODULE_PATH, normalizeSlashes(combinePaths(consts.LIB, consts.SITE_PACKAGES)));
 export const typeshedFolder = combinePaths(MODULE_PATH, normalizeSlashes(consts.TYPESHED_FALLBACK));
 export const srcFolder = normalizeSlashes('/.src');