Skip to content

Commit

Permalink
Added the ability to specify file-level overrides to config settings …
Browse files Browse the repository at this point in the history
…using #pyright comment.
  • Loading branch information
msfterictraut committed May 21, 2019
1 parent 98cb332 commit d90609c
Show file tree
Hide file tree
Showing 10 changed files with 354 additions and 219 deletions.
7 changes: 4 additions & 3 deletions server/src/analyzer/analyzerFileInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* Input type common to multiple analyzer passes.
*/

import { ConfigOptions, ExecutionEnvironment } from '../common/configOptions';
import { ConfigOptions, DiagnosticSettings, ExecutionEnvironment } from '../common/configOptions';
import { ConsoleInterface } from '../common/console';
import { TextRangeDiagnosticSink } from '../common/diagnosticSink';
import { TextRange } from '../common/textRange';
Expand All @@ -24,8 +24,9 @@ export interface AnalyzerFileInfo {
typingModulePath?: string;
diagnosticSink: TextRangeDiagnosticSink;
executionEnvironment: ExecutionEnvironment;
configOptions: ConfigOptions;
useStrictMode: boolean;
// configOptions: ConfigOptions;
diagnosticSettings: DiagnosticSettings;
// useStrictMode: boolean;
lines: TextRangeCollection<TextRange>;
filePath: string;
isStubFile: boolean;
Expand Down
94 changes: 76 additions & 18 deletions server/src/analyzer/commentUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,38 +8,96 @@
* or other directives from them.
*/

import { cloneDiagnosticSettings, DiagnosticLevel, DiagnosticSettings,
getBooleanDiagnosticSettings, getDiagLevelSettings, getStrictDiagnosticSettings } from '../common/configOptions';
import { TextRangeCollection } from '../common/textRangeCollection';
import { Token } from '../parser/tokenizerTypes';

export interface FileLevelDirectives {
useStrictMode: boolean;
}

export class CommentUtils {
static getFileLevelDirectives(tokens: TextRangeCollection<Token>): FileLevelDirectives {
let directives: FileLevelDirectives = {
useStrictMode: false
};
static getFileLevelDirectives(tokens: TextRangeCollection<Token>,
defaultSettings: DiagnosticSettings): DiagnosticSettings {

let settings = cloneDiagnosticSettings(defaultSettings);

for (let i = 0; i < tokens.count; i++) {
const token = tokens.getItemAt(i);
if (token.comments) {
for (const comment of token.comments) {
const value = comment.value.trim();

// Is this a pyright-specific comment?
const pyrightPrefix = 'pyright:';
if (value.startsWith(pyrightPrefix)) {
const operand = value.substr(pyrightPrefix.length).trim();

if (operand === 'strict') {
directives.useStrictMode = true;
}
}
settings = this._parsePyrightComment(value, settings);
}
}
}

return directives;
return settings;
}

private static _parsePyrightComment(commentValue: string, settings: DiagnosticSettings) {
// Is this a pyright-specific comment?
const pyrightPrefix = 'pyright:';
if (commentValue.startsWith(pyrightPrefix)) {
const operands = commentValue.substr(pyrightPrefix.length).trim();
const operandList = operands.split(',').map(s => s.trim());

// If it contains a "strict" operand, replace the existing
// diagnostic settings with their strict counterparts.
if (operandList.some(s => s === 'strict')) {
settings = getStrictDiagnosticSettings();
}

for (let operand of operandList) {
settings = this._parsePyrightOperand(operand, settings);
}
}

return settings;
}

private static _parsePyrightOperand(operand: string, settings: DiagnosticSettings) {
const operandSplit = operand.split('=').map(s => s.trim());
if (operandSplit.length !== 2) {
return settings;
}

const settingName = operandSplit[0];
const boolSettings = getBooleanDiagnosticSettings();
const diagLevelSettings = getDiagLevelSettings();

if (diagLevelSettings.find(s => s === settingName)) {
const diagLevelValue = this._parseDiagLevel(operandSplit[1]);
if (diagLevelValue !== undefined) {
(settings as any)[settingName] = diagLevelValue;
}
} else if (boolSettings.find(s => s === settingName)) {
const boolValue = this._parseBoolSetting(operandSplit[1]);
if (boolValue !== undefined) {
(settings as any)[settingName] = boolValue;
}
}

return settings;
}

private static _parseDiagLevel(value: string): DiagnosticLevel | undefined {
if (value === 'false' || value === 'none') {
return 'none';
} else if (value === 'warning') {
return 'warning';
} else if (value === 'true' || value === 'error') {
return 'error';
}

return undefined;
}

private static _parseBoolSetting(value: string): boolean | undefined {
if (value === 'false') {
return false;
} else if (value === 'true') {
return true;
}

return undefined;
}
}
22 changes: 11 additions & 11 deletions server/src/analyzer/expressionEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ export class ExpressionEvaluator {

if (type instanceof UnionType && type.getTypes().some(t => t instanceof NoneType)) {
this._addDiagnostic(
this._fileInfo.configOptions.reportOptionalIterable,
this._fileInfo.diagnosticSettings.reportOptionalIterable,
`Object of type 'None' cannot be used as iterable value`,
errorNode);
type = TypeUtils.removeNoneFromUnion(type);
Expand Down Expand Up @@ -733,7 +733,7 @@ export class ExpressionEvaluator {
baseType.getTypes().forEach(typeEntry => {
if (typeEntry instanceof NoneType) {
this._addDiagnostic(
this._fileInfo.configOptions.reportOptionalMemberAccess,
this._fileInfo.diagnosticSettings.reportOptionalMemberAccess,
`'${ memberName }' is not a known member of 'None'`, node.memberName);
} else {
let typeResult = this._getTypeFromMemberAccessExpressionWithBaseType(node,
Expand Down Expand Up @@ -986,7 +986,7 @@ export class ExpressionEvaluator {
return this._getTypeFromIndexedObject(node, subtype, usage);
} else if (subtype instanceof NoneType) {
this._addDiagnostic(
this._fileInfo.configOptions.reportOptionalSubscript,
this._fileInfo.diagnosticSettings.reportOptionalSubscript,
`Optional of type 'None' cannot be subscripted`,
node.baseExpression);

Expand Down Expand Up @@ -1221,7 +1221,7 @@ export class ExpressionEvaluator {
// as a function rather than a class, so we need to check for it here.
if (callType.getBuiltInName() === 'namedtuple') {
this._addDiagnostic(
this._fileInfo.configOptions.reportUntypedNamedTuple,
this._fileInfo.diagnosticSettings.reportUntypedNamedTuple,
`'namedtuple' provides no types for tuple entries. Use 'NamedTuple' instead.`,
errorNode);
type = this._createNamedTupleType(errorNode, argList, false, cachedCallType);
Expand Down Expand Up @@ -1273,7 +1273,7 @@ export class ExpressionEvaluator {
callType.getTypes().forEach(typeEntry => {
if (typeEntry instanceof NoneType) {
this._addDiagnostic(
this._fileInfo.configOptions.reportOptionalCall,
this._fileInfo.diagnosticSettings.reportOptionalCall,
`Object of type 'None' cannot be called`,
errorNode);
} else {
Expand Down Expand Up @@ -1441,7 +1441,7 @@ export class ExpressionEvaluator {
for (let type of callType.getTypes()) {
if (type instanceof NoneType) {
this._addDiagnostic(
this._fileInfo.configOptions.reportOptionalCall,
this._fileInfo.diagnosticSettings.reportOptionalCall,
`Object of type 'None' cannot be called`,
errorNode);
} else {
Expand Down Expand Up @@ -2055,7 +2055,7 @@ export class ExpressionEvaluator {
if (node.operator !== OperatorType.Not) {
if (TypeUtils.isOptionalType(exprType)) {
this._addDiagnostic(
this._fileInfo.configOptions.reportOptionalOperand,
this._fileInfo.diagnosticSettings.reportOptionalOperand,
`Operator '${ ParseTreeUtils.printOperator(node.operator) }' not ` +
`supported for 'None' type`,
node.expression);
Expand Down Expand Up @@ -2150,7 +2150,7 @@ export class ExpressionEvaluator {
// None is a valid operand for these operators.
if (node.operator !== OperatorType.Equals && node.operator !== OperatorType.NotEquals) {
this._addDiagnostic(
this._fileInfo.configOptions.reportOptionalOperand,
this._fileInfo.diagnosticSettings.reportOptionalOperand,
`Operator '${ ParseTreeUtils.printOperator(node.operator) }' not ` +
`supported for 'None' type`,
node.leftExpression);
Expand Down Expand Up @@ -2408,7 +2408,7 @@ export class ExpressionEvaluator {
// are the same type, we'll assume that all values in this dictionary should
// be the same.
if (valueTypes.length > 0) {
if (this._fileInfo.configOptions.strictDictionaryInference || this._fileInfo.useStrictMode) {
if (this._fileInfo.diagnosticSettings.strictDictionaryInference) {
valueType = TypeUtils.combineTypes(valueTypes);
} else {
valueType = TypeUtils.areTypesSame(valueTypes) ? valueTypes[0] : UnknownType.create();
Expand All @@ -2433,7 +2433,7 @@ export class ExpressionEvaluator {
entry => TypeUtils.stripLiteralValue(this.getType(entry)));

if (entryTypes.length > 0) {
if (this._fileInfo.configOptions.strictListInference || this._fileInfo.useStrictMode) {
if (this._fileInfo.diagnosticSettings.strictListInference) {
listEntryType = TypeUtils.combineTypes(entryTypes);
} else {
// Is the list homogeneous? If so, use stricter rules. Otherwise relax the rules.
Expand Down Expand Up @@ -3159,7 +3159,7 @@ export class ExpressionEvaluator {
}

private _addDiagnostic(diagLevel: DiagnosticLevel, message: string, textRange: TextRange) {
if (diagLevel === 'error' || this._fileInfo.useStrictMode) {
if (diagLevel === 'error') {
this._addError(message, textRange);
} else if (diagLevel === 'warning') {
this._addWarning(message, textRange);
Expand Down
4 changes: 2 additions & 2 deletions server/src/analyzer/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ export class Program {
Object.keys(closureMap).forEach(filePath => {
assert(!this._sourceFileMap[filePath].sourceFile.isAnalysisFinalized());

if (options.reportImportCycles !== 'none') {
if (options.diagnosticSettings.reportImportCycles !== 'none') {
this._detectAndReportImportCycles(this._sourceFileMap[filePath]);
}

Expand Down Expand Up @@ -756,7 +756,7 @@ export class Program {
});
}
} else if (options.verboseOutput) {
if (!sourceFileInfo.isTypeshedFile || options.reportTypeshedErrors) {
if (!sourceFileInfo.isTypeshedFile || options.diagnosticSettings.reportTypeshedErrors) {
this._console.log(`Could not import '${ importResult.importName }' ` +
`in file '${ sourceFileInfo.sourceFile.getFilePath() }'`);
if (importResult.importFailureInfo) {
Expand Down
8 changes: 4 additions & 4 deletions server/src/analyzer/semanticAnalyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,11 @@ export abstract class SemanticAnalyzer extends ParseTreeWalker {
assert(importResult !== undefined);
if (importResult) {
if (!importResult.importFound) {
this._addDiagnostic(this._fileInfo.configOptions.reportMissingImports,
this._addDiagnostic(this._fileInfo.diagnosticSettings.reportMissingImports,
`Import '${ importResult.importName }' could not be resolved`, node);
} else if (importResult.importType === ImportType.ThirdParty) {
if (!importResult.isStubFile) {
this._addDiagnostic(this._fileInfo.configOptions.reportMissingTypeStubs,
this._addDiagnostic(this._fileInfo.diagnosticSettings.reportMissingTypeStubs,
`Stub file not found for '${ importResult.importName }'`, node);
}
}
Expand Down Expand Up @@ -370,7 +370,7 @@ export abstract class SemanticAnalyzer extends ParseTreeWalker {
if (stringToken.invalidEscapeOffsets) {
stringToken.invalidEscapeOffsets.forEach(offset => {
const textRange = new TextRange(stringToken.start + offset, 1);
this._addDiagnostic(this._fileInfo.configOptions.reportInvalidStringEscapeSequence,
this._addDiagnostic(this._fileInfo.diagnosticSettings.reportInvalidStringEscapeSequence,
'Unsupported escape sequence in string literal', textRange);
});
}
Expand Down Expand Up @@ -495,7 +495,7 @@ export abstract class SemanticAnalyzer extends ParseTreeWalker {
}

private _addDiagnostic(diagLevel: DiagnosticLevel, message: string, textRange: TextRange) {
if (diagLevel === 'error' || this._fileInfo.useStrictMode) {
if (diagLevel === 'error') {
this._addError(message, textRange);
} else if (diagLevel === 'warning') {
this._addWarning(message, textRange);
Expand Down
27 changes: 15 additions & 12 deletions server/src/analyzer/sourceFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import * as assert from 'assert';
import * as fs from 'fs';
import { CompletionList } from 'vscode-languageserver';

import { ConfigOptions, ExecutionEnvironment } from '../common/configOptions';
import { cloneDiagnosticSettings, ConfigOptions, DiagnosticSettings,
ExecutionEnvironment,
getStrictDiagnosticSettings } from '../common/configOptions';
import { ConsoleInterface, StandardConsole } from '../common/console';
import { Diagnostic, DiagnosticCategory, DiagnosticTextPosition, DocumentTextRange } from '../common/diagnostic';
import { DiagnosticSink, TextRangeDiagnosticSink } from '../common/diagnosticSink';
Expand All @@ -26,7 +28,7 @@ import { TestWalker } from '../tests/testWalker';
import { AnalyzerFileInfo, ImportMap } from './analyzerFileInfo';
import { AnalyzerNodeInfo } from './analyzerNodeInfo';
import { CircularDependency } from './circularDependency';
import { CommentUtils, FileLevelDirectives } from './commentUtils';
import { CommentUtils } from './commentUtils';
import { CompletionProvider } from './completionProvider';
import { DefinitionProvider } from './definitionProvider';
import { HoverProvider } from './hoverProvider';
Expand Down Expand Up @@ -55,13 +57,14 @@ export interface AnalysisJob {
nextPhaseToRun: AnalysisPhase;
parseTreeNeedsCleaning: boolean;
parseResults?: ParseResults;
fileLevelDirectives?: FileLevelDirectives;

parseDiagnostics: Diagnostic[];
semanticAnalysisDiagnostics: Diagnostic[];
typeAnalysisLastPassDiagnostics: Diagnostic[];
typeAnalysisFinalDiagnostics: Diagnostic[];

diagnosticSettings: DiagnosticSettings;

circularDependencies: CircularDependency[];
hitMaxImportDepth?: number;

Expand Down Expand Up @@ -109,6 +112,8 @@ export class SourceFile {
typeAnalysisLastPassDiagnostics: [],
typeAnalysisFinalDiagnostics: [],

diagnosticSettings: getStrictDiagnosticSettings(),

circularDependencies: [],

typeAnalysisPassNumber: 1,
Expand Down Expand Up @@ -189,8 +194,8 @@ export class SourceFile {
this._analysisJob.semanticAnalysisDiagnostics,
this._analysisJob.typeAnalysisFinalDiagnostics);

if (options.reportImportCycles !== 'none' && this._analysisJob.circularDependencies.length > 0) {
const category = options.reportImportCycles === 'warning' ?
if (options.diagnosticSettings.reportImportCycles !== 'none' && this._analysisJob.circularDependencies.length > 0) {
const category = options.diagnosticSettings.reportImportCycles === 'warning' ?
DiagnosticCategory.Warning : DiagnosticCategory.Error;

this._analysisJob.circularDependencies.forEach(cirDep => {
Expand All @@ -205,9 +210,9 @@ export class SourceFile {
}

if (this._isTypeshedStubFile) {
if (options.reportTypeshedErrors === 'none') {
if (options.diagnosticSettings.reportTypeshedErrors === 'none') {
return undefined;
} else if (options.reportTypeshedErrors === 'warning') {
} else if (options.diagnosticSettings.reportTypeshedErrors === 'warning') {
// Convert all the errors to warnings.
diagList = diagList.map(diag => {
if (diag.category === DiagnosticCategory.Error) {
Expand Down Expand Up @@ -416,8 +421,8 @@ export class SourceFile {
this._resolveImports(walker.getImportedModules(), configOptions, execEnvironment);
this._analysisJob.parseDiagnostics = diagSink.diagnostics;

this._analysisJob.fileLevelDirectives = CommentUtils.getFileLevelDirectives(
this._analysisJob.parseResults.tokens);
this._analysisJob.diagnosticSettings = CommentUtils.getFileLevelDirectives(
this._analysisJob.parseResults.tokens, configOptions.diagnosticSettings);
} catch (e) {
let message: string;
if (e instanceof Error) {
Expand Down Expand Up @@ -616,9 +621,7 @@ export class SourceFile {
typingModulePath: this._analysisJob.typingModulePath,
diagnosticSink: analysisDiagnostics,
executionEnvironment: configOptions.findExecEnvironment(this._filePath),
configOptions,
useStrictMode: !!this._analysisJob.fileLevelDirectives &&
this._analysisJob.fileLevelDirectives.useStrictMode,
diagnosticSettings: this._analysisJob.diagnosticSettings,
lines: this._analysisJob.parseResults!.lines,
filePath: this._filePath,
isStubFile: this._isStubFile,
Expand Down
Loading

0 comments on commit d90609c

Please sign in to comment.