Skip to content

Commit

Permalink
Remove linting on change
Browse files Browse the repository at this point in the history
  • Loading branch information
@karthiknadig
karthiknadig committed Jan 24, 2025
1 parent 46e222b commit 118b683
Show file tree
Hide file tree
Showing 8 changed files with 2 additions and 185 deletions.
2 changes: 0 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ The Pylint extension provides a series of features to help your productivity whi

- **Integrated Linting**: Once this extension is installed in Visual Studio Code, Pylint is automatically executed when you open a Python file, providing immediate feedback on your code quality.
- **Customizable Pylint Version**: By default, this extension uses the version of Pylint that is shipped with the extension. However, you can configure it to use a different binary installed in your environment through the `pylint.importStrategy` setting, or set it to a custom Pylint executable through the `pylint.path` settings.
- **Immediate Feedback**: By default, Pylint will update the diagnostics in the editor once you save the file. But you can get immediate feedback on your code quality as you type by enabling the `pylint.lintOnChange` setting.
- **Mono repo support**: If you are working with a mono repo, you can configure the extension to lint Python files in subfolders of the workspace root folder by setting the `pylint.cwd` setting to `${fileDirname}`. You can also set it to ignore/skip linting for certain files or folder paths by specifying a glob pattern to the `pylint.ignorePatterns` setting.
- **Customizable Linting Rules**: You can customize the severity of specific Pylint error codes through the `pylint.severity` setting.

Expand All @@ -38,7 +37,6 @@ There are several settings you can configure to customize the behavior of this e
| pylint.interpreter | `[]` | Path to a Python executable or a command that will be used to launch the Pylint server and any subprocess. Accepts an array of a single or multiple strings. When set to `[]`, the extension will use the path to the selected Python interpreter. If passing a command, each argument should be provided as a separate string in the array. |
| pylint.importStrategy | `useBundled` | Defines which Pylint binary to be used to lint Python files. When set to `useBundled`, the extension will use the Pylint binary that is shipped with the extension. When set to `fromEnvironment`, the extension will attempt to use the Pylint binary and all dependencies that are available in the currently selected environment. Note: If the extension can't find a valid Pylint binary in the selected environment, it will fallback to using the Pylint binary that is shipped with the extension. This setting will be overriden if `pylint.path` is set. |
| pylint.showNotification | `off` | Controls when notifications are shown by this extension. Accepted values are `onError`, `onWarning`, `always` and `off`. |
| pylint.lintOnChange | `false` | Enable linting Python files with Pylint as you type. |
| pylint.ignorePatterns | `[]` | Configure [glob patterns](https://docs.python.org/3/library/fnmatch.html) as supported by the fnmatch Python library to exclude files or folders from being linted with Pylint. |

The following variables are supported for substitution in the `pylint.args`, `pylint.cwd`, `pylint.path`, `pylint.interpreter` and `pylint.ignorePatterns` settings:
Expand Down
11 changes: 0 additions & 11 deletions bundled/tool/lsp_server.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,17 +120,6 @@ def did_close(params: lsp.DidCloseTextDocumentParams) -> None:
# Publishing empty diagnostics to clear the entries for this file.
LSP_SERVER.publish_diagnostics(document.uri, [])


if os.getenv("VSCODE_PYLINT_LINT_ON_CHANGE"):

@LSP_SERVER.feature(lsp.TEXT_DOCUMENT_DID_CHANGE)
def did_change(params: lsp.DidChangeTextDocumentParams) -> None:
"""LSP handler for textDocument/didChange request."""
document = LSP_SERVER.workspace.get_document(params.text_document.uri)
diagnostics: list[lsp.Diagnostic] = _linting_helper(document)
LSP_SERVER.publish_diagnostics(document.uri, diagnostics)


def _linting_helper(document: workspace.Document) -> list[lsp.Diagnostic]:
try:
extra_args = []
Expand Down
9 changes: 0 additions & 9 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,15 +139,6 @@
},
"type": "array"
},
"pylint.lintOnChange": {
"default": false,
"markdownDescription": "%settings.lintOnChange.description%",
"scope": "machine",
"type": "boolean",
"tags": [
"experimental"
]
},
"pylint.path": {
"default": [],
"markdownDescription": "%settings.path.description%",
Expand Down
1 change: 0 additions & 1 deletion package.nls.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"settings.cwd.description": "Sets the current working directory used to lint Python files with Pylint. By default, it uses the root directory of the workspace `${workspaceFolder}`. You can set it to `${fileDirname}` to use the parent folder of the file being linted as the working directory for Pylint.",
"settings.enabled.description": "Enable/disable linting Python files with Pylint.",
"settings.severity.description": "Mapping of Pylint's message types to VS Code's diagnostic severity levels as displayed in the Problems window. You can also use it to override specific Pylint error codes. \n Example:</br> `{\"convention\": \"Information\", \"error\": \"Error\", \"fatal\": \"Error\", \"refactor\": \"Hint\", \"warning\": \"Warning\", \"W0611\": \"Error\", \"undefined-variable\": \"Warning\"}`",
"settings.lintOnChange.description": "Enable linting Python files with Pylint as you type.",
"settings.path.description": "Path or command to be used by the extension to lint Python files with Pylint. Accepts an array of a single or multiple strings. If passing a command, each argument should be provided as a separate string in the array. If set to `[\"pylint\"]`, it will use the version of Pylint available in the `PATH` environment variable. Note: Using this option may slowdown linting. \nExamples: \n- `[\"~/global_env/pylint\"]` \n- `[\"conda\", \"run\", \"-n\", \"lint_env\", \"python\", \"-m\", \"pylint\"]` \n `[\"pylint\"]`",
"settings.ignorePatterns.description": "Configure [glob patterns](https://docs.python.org/3/library/fnmatch.html) as supported by the fnmatch Python library to exclude files or folders from being linted with Pylint.",
"settings.importStrategy.description": "Defines which Pylint binary to be used to lint Python files. When set to `useBundled`, the extension will use the Pylint binary that is shipped with the extension. When set to `fromEnvironment`, the extension will attempt to use the Pylint binary and all dependencies that are available in the currently selected environment. Note: If the extension can't find a valid Pylint binary in the selected environment, it will fallback to using the Pylint binary that is shipped with the extension The `pylint.path` setting may also be ignored when this setting is set to `fromEnvironment`.",
Expand Down
6 changes: 1 addition & 5 deletions src/common/server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
import { DEBUG_SERVER_SCRIPT_PATH, SERVER_SCRIPT_PATH } from './constants';
import { traceError, traceInfo, traceVerbose } from './logging';
import { getDebuggerPath } from './python';
import { getExtensionSettings, getGlobalSettings, ISettings, isLintOnChangeEnabled } from './settings';
import { getExtensionSettings, getGlobalSettings, ISettings } from './settings';
import { getLSClientTraceLevel, getDocumentSelector } from './utilities';
import { updateStatus } from './status';

Expand Down Expand Up @@ -47,10 +47,6 @@ async function createServer(

newEnv.PYTHONUTF8 = '1';

if (isLintOnChangeEnabled(serverId)) {
newEnv.VSCODE_PYLINT_LINT_ON_CHANGE = '1';
}

const args =
newEnv.USE_DEBUGPY === 'False' || !isDebugScript
? settings.interpreter.slice(1).concat([SERVER_SCRIPT_PATH])
Expand Down
6 changes: 0 additions & 6 deletions src/common/settings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,6 @@ export async function getGlobalSettings(namespace: string, includeInterpreter?:
return setting;
}

export function isLintOnChangeEnabled(namespace: string): boolean {
const config = getConfiguration(namespace);
return config.get<boolean>('lintOnChange', false);
}

export function checkIfConfigurationChanged(e: ConfigurationChangeEvent, namespace: string): boolean {
const settings = [
`${namespace}.args`,
Expand All @@ -195,7 +190,6 @@ export function checkIfConfigurationChanged(e: ConfigurationChangeEvent, namespa
`${namespace}.importStrategy`,
`${namespace}.showNotifications`,
`${namespace}.ignorePatterns`,
`${namespace}.lintOnChange`,
'python.analysis.extraPaths',
];
const changed = settings.map((s) => e.affectsConfiguration(s));
Expand Down
140 changes: 0 additions & 140 deletions src/test/python_tests/test_linting.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,146 +250,6 @@ def _handler(params):
assert_that(actual, is_(expected))


def test_publish_diagnostics_on_change():
"""Test to ensure diagnostic clean-up on file close."""
contents = TEST_FILE2_PATH.read_text(encoding="utf-8")

actual = []
os.environ["VSCODE_PYLINT_LINT_ON_CHANGE"] = "1"
with session.LspSession() as ls_session:
ls_session.initialize()

done = Event()

def _handler(params):
nonlocal actual
actual = params
done.set()

ls_session.set_notification_callback(session.PUBLISH_DIAGNOSTICS, _handler)

ls_session.notify_did_open(
{
"textDocument": {
"uri": TEST_FILE2_URI,
"languageId": "python",
"version": 1,
"text": contents,
}
}
)
# wait for some time to receive all notifications
done.wait(TIMEOUT)

# We should receive empty diagnostics
assert_that(
actual,
is_(
{
"uri": TEST_FILE2_URI,
"diagnostics": [],
}
),
)

# reset waiting
done.clear()

# make code change with linting errors
ls_session.notify_did_change(
{
"textDocument": {
"uri": TEST_FILE2_URI,
"version": 1,
},
"contentChanges": [
{
"range": {
"start": {"line": 1, "character": 0},
"end": {"line": 1, "character": 0},
},
"text": "print(x)",
}
],
}
)

# wait for some time to receive all notifications
done.wait(TIMEOUT)

expected = {
"uri": TEST_FILE2_URI,
"diagnostics": [
{
"range": {
"start": {"line": 1, "character": 0},
"end": {"line": 1, "character": 0},
},
"message": "Final newline missing",
"severity": 3,
"code": "C0304:missing-final-newline",
"codeDescription": {
"href": f"{DOCUMENTATION_HOME}/convention/missing-final-newline.html"
},
"source": "Pylint",
},
{
"range": {
"start": {"line": 1, "character": 6},
"end": {
"line": 1,
"character": 7,
},
},
"message": "Undefined variable 'x'",
"severity": 1,
"code": "E0602:undefined-variable",
"codeDescription": {
"href": f"{DOCUMENTATION_HOME}/error/undefined-variable.html"
},
"source": "Pylint",
},
],
}
assert_that(actual, is_(expected))

# reset waiting
done.clear()

# make code change fixing linting errors
ls_session.notify_did_change(
{
"textDocument": {
"uri": TEST_FILE2_URI,
"version": 1,
},
"contentChanges": [
{
"range": {
"start": {"line": 1, "character": 0},
"end": {"line": 2, "character": 0},
},
"text": "print('hello')\n",
}
],
}
)

# wait for some time to receive all notifications
done.wait(TIMEOUT)

# We should receive empty diagnostics
assert_that(
actual,
is_(
{
"uri": TEST_FILE2_URI,
"diagnostics": [],
}
),
)


@pytest.mark.parametrize("lint_code", ["W0611", "unused-import", "warning"])
def test_severity_setting(lint_code):
"""Test to ensure linting on file open."""
Expand Down
12 changes: 1 addition & 11 deletions src/test/ts_tests/tests/common/settings.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as TypeMoq from 'typemoq';
import { Uri, WorkspaceConfiguration, WorkspaceFolder } from 'vscode';
import { EXTENSION_ROOT_DIR } from '../../../../common/constants';
import * as python from '../../../../common/python';
import { ISettings, getWorkspaceSettings, isLintOnChangeEnabled } from '../../../../common/settings';
import { ISettings, getWorkspaceSettings } from '../../../../common/settings';
import * as vscodeapi from '../../../../common/vscodeapi';

// eslint-disable-next-line @typescript-eslint/naming-convention
Expand Down Expand Up @@ -297,15 +297,5 @@ suite('Settings Tests', () => {
configMock.verifyAll();
pythonConfigMock.verifyAll();
});

[true, false].forEach((value) => {
test(`Lint on change settings: ${value}`, async () => {
configMock
.setup((c) => c.get<boolean>('lintOnChange', false))
.returns(() => value)
.verifiable(TypeMoq.Times.atLeastOnce());
assert.deepStrictEqual(isLintOnChangeEnabled('pylint'), value);
});
});
});
});

0 comments on commit 118b683

Please sign in to comment.