Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display lsp diagnostics in console #1406

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7beeaff
Manually merge transpile-using-lsp-saved into transpile-unsing-lsp
ericvergnaud Jan 6, 2025
879407c
fix merge issue
ericvergnaud Jan 6, 2025
b9e663e
move lineage stuff out of LSP api
ericvergnaud Jan 6, 2025
3bc5fb2
refactor TranspileError and TranspileStatus
ericvergnaud Jan 6, 2025
8cb310f
fix refactoring issues
ericvergnaud Jan 6, 2025
8ac675a
drop work in progress
ericvergnaud Dec 19, 2024
819aad5
add range
ericvergnaud Dec 19, 2024
e8b7b3c
fix failing test
ericvergnaud Dec 19, 2024
1c36787
formatting
ericvergnaud Dec 19, 2024
203573e
fix merge issue
ericvergnaud Jan 6, 2025
47e6227
convert diagnostics into TranspileErrors
ericvergnaud Dec 20, 2024
8a58089
display transpile errors in console
ericvergnaud Dec 20, 2024
40b1eff
formatting
ericvergnaud Dec 20, 2024
6f38994
fix failing tests
ericvergnaud Dec 20, 2024
e301186
fix merge issue
ericvergnaud Dec 20, 2024
b02795f
successfully prints transpile errors in the console
ericvergnaud Dec 26, 2024
5ee7117
formatting + failing tests
ericvergnaud Dec 26, 2024
c4836f3
fix failing tests
ericvergnaud Dec 26, 2024
f7ad038
drop no longer relevant tests
ericvergnaud Jan 7, 2025
5baeb69
bump max args
ericvergnaud Jan 8, 2025
82a7bcb
add docs
ericvergnaud Jan 9, 2025
5a4ba11
fix typo
ericvergnaud Jan 9, 2025
46a41af
Merge branch 'bump-max-args' into feature/multiplexer/transpile-using…
ericvergnaud Jan 10, 2025
f840530
Merge branch 'feature/multiplexer/transpile-using-lsp' into feature/m…
ericvergnaud Jan 10, 2025
7dabd9a
Merge branch 'feature/multiplexer/refactor-transpile-error' into feat…
ericvergnaud Jan 10, 2025
0abeebf
Merge branch 'main' into feature/multiplexer/display-lsp-diagnostics-…
ericvergnaud Jan 22, 2025
6543b8f
fix merge issues
ericvergnaud Jan 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/databricks/labs/remorph/cli.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import asyncio
import json
import os
from pathlib import Path
Expand Down Expand Up @@ -85,8 +86,10 @@ def transpile(
mode=mode,
sdk_config=sdk_config,
)
status, errors = asyncio.run(do_transpile(ctx.workspace_client, engine, config))

status = do_transpile(ctx.workspace_client, engine, config)
for error in errors:
print(str(error))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
print(str(error))
can we use logger instead when run in cli only stderr is redirected to console.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit


print(json.dumps(status))

Expand Down
2 changes: 1 addition & 1 deletion src/databricks/labs/remorph/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def output_path(self):

@property
def error_path(self):
return None if self.error_file_path is None else Path(self.error_file_path)
return Path(self.error_file_path) if self.error_file_path else None

@property
def target_dialect(self):
Expand Down
43 changes: 27 additions & 16 deletions src/databricks/labs/remorph/transpiler/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import datetime
import logging
from pathlib import Path
from typing import cast
from typing import cast, Any

from databricks.labs.remorph.__about__ import __version__
from databricks.labs.remorph.config import (
Expand Down Expand Up @@ -33,7 +33,7 @@
logger = logging.getLogger(__name__)


def _process_file(
async def _process_file(
config: TranspileConfig,
validator: Validator | None,
transpiler: TranspileEngine,
Expand All @@ -46,8 +46,8 @@ def _process_file(
with input_path.open("r") as f:
source_sql = remove_bom(f.read())

transpile_result = asyncio.run(
_transpile(transpiler, config.source_dialect, config.target_dialect, source_sql, input_path)
transpile_result = await _transpile(
transpiler, config.source_dialect, config.target_dialect, source_sql, input_path
)
error_list.extend(transpile_result.error_list)

Expand All @@ -71,7 +71,7 @@ def _process_file(
return transpile_result.success_count, error_list


def _process_directory(
async def _process_directory(
config: TranspileConfig,
validator: Validator | None,
transpiler: TranspileEngine,
Expand All @@ -93,14 +93,14 @@ def _process_directory(
continue

output_file_name = output_folder_base / file.name
success_count, error_list = _process_file(config, validator, transpiler, file, output_file_name)
success_count, error_list = await _process_file(config, validator, transpiler, file, output_file_name)
counter = counter + success_count
all_errors.extend(error_list)

return counter, all_errors


def _process_input_dir(config: TranspileConfig, validator: Validator | None, transpiler: TranspileEngine):
async def _process_input_dir(config: TranspileConfig, validator: Validator | None, transpiler: TranspileEngine):
error_list = []
file_list = []
counter = 0
Expand All @@ -112,13 +112,13 @@ def _process_input_dir(config: TranspileConfig, validator: Validator | None, tra
msg = f"Processing for sqls under this folder: {folder}"
logger.info(msg)
file_list.extend(files)
no_of_sqls, errors = _process_directory(config, validator, transpiler, root, base_root, files)
no_of_sqls, errors = await _process_directory(config, validator, transpiler, root, base_root, files)
counter = counter + no_of_sqls
error_list.extend(errors)
return TranspileStatus(file_list, counter, error_list)


def _process_input_file(
async def _process_input_file(
config: TranspileConfig, validator: Validator | None, transpiler: TranspileEngine
) -> TranspileStatus:
if not is_sql_file(config.input_path):
Expand All @@ -135,12 +135,23 @@ def _process_input_file(

make_dir(output_path)
output_file = output_path / config.input_path.name
no_of_sqls, error_list = _process_file(config, validator, transpiler, config.input_path, output_file)
no_of_sqls, error_list = await _process_file(config, validator, transpiler, config.input_path, output_file)
return TranspileStatus([config.input_path], no_of_sqls, error_list)


@timeit
def transpile(workspace_client: WorkspaceClient, engine: TranspileEngine, config: TranspileConfig):
async def transpile(
workspace_client: WorkspaceClient, engine: TranspileEngine, config: TranspileConfig
) -> tuple[list[dict[str, Any]], list[TranspileError]]:
await engine.initialize(config)
status, errors = await _do_transpile(workspace_client, engine, config)
await engine.shutdown()
return status, errors


async def _do_transpile(
workspace_client: WorkspaceClient, engine: TranspileEngine, config: TranspileConfig
) -> tuple[list[dict[str, Any]], list[TranspileError]]:
"""
[Experimental] Transpiles the SQL queries from one dialect to another.

Expand All @@ -162,9 +173,9 @@ def transpile(workspace_client: WorkspaceClient, engine: TranspileEngine, config
if config.input_source is None:
raise InvalidInputException("Missing input source!")
if config.input_path.is_dir():
result = _process_input_dir(config, validator, engine)
result = await _process_input_dir(config, validator, engine)
elif config.input_path.is_file():
result = _process_input_file(config, validator, engine)
result = await _process_input_file(config, validator, engine)
else:
msg = f"{config.input_source} does not exist."
logger.error(msg)
Expand Down Expand Up @@ -194,7 +205,7 @@ def transpile(workspace_client: WorkspaceClient, engine: TranspileEngine, config
"error_log_file": str(error_log_path),
}
)
return status
return status, result.error_list


def verify_workspace_client(workspace_client: WorkspaceClient) -> WorkspaceClient:
Expand All @@ -213,9 +224,9 @@ def verify_workspace_client(workspace_client: WorkspaceClient) -> WorkspaceClien


async def _transpile(
transpiler: TranspileEngine, from_dialect: str, to_dialect: str, source_code: str, input_path: Path
engine: TranspileEngine, from_dialect: str, to_dialect: str, source_code: str, input_path: Path
) -> TranspileResult:
return await transpiler.transpile(from_dialect, to_dialect, source_code, input_path)
return await engine.transpile(from_dialect, to_dialect, source_code, input_path)


def _validation(
Expand Down
65 changes: 59 additions & 6 deletions src/databricks/labs/remorph/transpiler/lsp/lsp_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@
TextDocumentIdentifier,
METHOD_TO_TYPES,
LanguageKind,
Range as LSPRange,
Position as LSPPosition,
_SPECIAL_PROPERTIES,
DiagnosticSeverity,
)
from pygls.lsp.client import BaseLanguageClient
from pygls.exceptions import FeatureRequestError
Expand All @@ -40,7 +43,13 @@
from databricks.labs.remorph.config import TranspileConfig, TranspileResult
from databricks.labs.remorph.errors.exceptions import IllegalStateException
from databricks.labs.remorph.transpiler.transpile_engine import TranspileEngine
from databricks.labs.remorph.transpiler.transpile_status import TranspileError, ErrorSeverity, ErrorKind
from databricks.labs.remorph.transpiler.transpile_status import (
TranspileError,
ErrorKind,
ErrorSeverity,
CodeRange,
CodePosition,
)

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -187,15 +196,18 @@ def wrapper(params):
class ChangeManager(abc.ABC):

@classmethod
def apply(cls, source_code: str, file_path: Path, changes: Sequence[TextEdit]) -> TranspileResult:
if not changes:
def apply(
cls, source_code: str, changes: Sequence[TextEdit], diagnostics: Sequence[Diagnostic], file_path: Path
) -> TranspileResult:
if not changes and not diagnostics:
return TranspileResult(source_code, 1, [])
transpile_errors = [DiagnosticConverter.apply(file_path, diagnostic) for diagnostic in diagnostics]
try:
lines = source_code.split("\n")
for change in changes:
lines = cls._apply(lines, change)
transpiled_code = "\n".join(lines)
return TranspileResult(transpiled_code, 1, [])
return TranspileResult(transpiled_code, 1, transpile_errors)
except IndexError as e:
logger.error("Failed to apply changes", exc_info=e)
error = TranspileError(
Expand All @@ -205,7 +217,8 @@ def apply(cls, source_code: str, file_path: Path, changes: Sequence[TextEdit]) -
path=file_path,
message="Internal error, failed to apply changes",
)
return TranspileResult(source_code, 1, [error])
transpile_errors.append(error)
return TranspileResult(source_code, 1, transpile_errors)

@classmethod
def _apply(cls, lines: list[str], change: TextEdit) -> list[str]:
Expand Down Expand Up @@ -247,6 +260,46 @@ def _is_full_document_change(cls, lines: list[str], change: TextEdit) -> bool:
)


class DiagnosticConverter(abc.ABC):

_KIND_NAMES = {e.name for e in ErrorKind}

@classmethod
def apply(cls, file_path: Path, diagnostic: Diagnostic) -> TranspileError:
code = str(diagnostic.code)
kind = ErrorKind.INTERNAL
parts = code.split("-")
if len(parts) >= 2 and parts[0] in cls._KIND_NAMES:
kind = ErrorKind[parts[0]]
parts.pop(0)
code = "-".join(parts)
severity = cls._convert_severity(diagnostic.severity)
lsp_range = cls._convert_range(diagnostic.range)
return TranspileError(
code=code, kind=kind, severity=severity, path=file_path, message=diagnostic.message, range=lsp_range
)

@classmethod
def _convert_range(cls, lsp_range: LSPRange | None) -> CodeRange | None:
if not lsp_range:
return None
return CodeRange(cls._convert_position(lsp_range.start), cls._convert_position(lsp_range.end))

@classmethod
def _convert_position(cls, lsp_position: LSPPosition) -> CodePosition:
return CodePosition(lsp_position.line, lsp_position.character)

@classmethod
def _convert_severity(cls, severity: DiagnosticSeverity | None) -> ErrorSeverity:
if severity == DiagnosticSeverity.Information:
return ErrorSeverity.INFO
if severity == DiagnosticSeverity.Warning:
return ErrorSeverity.WARNING
if severity == DiagnosticSeverity.Error:
return ErrorSeverity.ERROR
return ErrorSeverity.INFO


class LSPEngine(TranspileEngine):

@classmethod
Expand Down Expand Up @@ -335,7 +388,7 @@ async def transpile(
self.open_document(file_path, source_code=source_code)
response = await self.transpile_document(file_path)
self.close_document(file_path)
return ChangeManager.apply(source_code, file_path, response.changes)
return ChangeManager.apply(source_code, response.changes, response.diagnostics, file_path)

def open_document(self, file_path: Path, encoding="utf-8", source_code: str | None = None) -> None:
if source_code is None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from sqlglot.expressions import Expression
from sqlglot.tokens import Token, TokenType

from databricks.labs.remorph.config import TranspileResult
from databricks.labs.remorph.config import TranspileResult, TranspileConfig
from databricks.labs.remorph.helpers.string_utils import format_error_message
from databricks.labs.remorph.transpiler.sqlglot import lca_utils
from databricks.labs.remorph.transpiler.sqlglot.dialect_utils import get_dialect
Expand Down Expand Up @@ -68,6 +68,12 @@ def _partial_transpile(
problem_list.append(ParserProblem(parsed_expression.original_sql, error))
return transpiled_sqls, problem_list

async def initialize(self, config: TranspileConfig) -> None:
pass

async def shutdown(self) -> None:
pass

async def transpile(
self, source_dialect: str, target_dialect: str, source_code: str, file_path: Path
) -> TranspileResult:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import abc
from pathlib import Path

from databricks.labs.remorph.config import TranspileResult
from databricks.labs.remorph.config import TranspileResult, TranspileConfig


class TranspileEngine(abc.ABC):
Expand All @@ -24,6 +24,12 @@ def load_engine(cls, transpiler_config_path: Path) -> TranspileEngine:

return LSPEngine.from_config_path(transpiler_config_path)

@abc.abstractmethod
async def initialize(self, config: TranspileConfig) -> None: ...

@abc.abstractmethod
async def shutdown(self) -> None: ...

@abc.abstractmethod
async def transpile(
self, source_dialect: str, target_dialect: str, source_code: str, file_path: Path
Expand Down
1 change: 1 addition & 0 deletions tests/resources/lsp_transpiler/internal.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
create table stuff(name varchar(12))
41 changes: 34 additions & 7 deletions tests/resources/lsp_transpiler/lsp_server.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os
import sys
from collections.abc import Sequence
from pathlib import Path
from typing import Any, Literal
from uuid import uuid4

Expand All @@ -23,6 +24,7 @@
Position,
METHOD_TO_TYPES,
_SPECIAL_PROPERTIES,
DiagnosticSeverity,
)
from pygls.lsp.server import LanguageServer

Expand Down Expand Up @@ -112,14 +114,39 @@ async def did_initialize(self, init_params: InitializeParams) -> None:
def transpile_to_databricks(self, params: TranspileDocumentParams) -> TranspileDocumentResult:
source_sql = self.workspace.get_text_document(params.uri).source
source_lines = source_sql.split("\n")
transpiled_sql = source_sql.upper()
changes = [
TextEdit(
range=Range(start=Position(0, 0), end=Position(len(source_lines), len(source_lines[-1]))),
new_text=transpiled_sql,
range = Range(start=Position(0, 0), end=Position(len(source_lines), len(source_lines[-1])))
transpiled_sql, diagnostics = self._transpile(Path(params.uri).name, range, source_sql)
changes = [TextEdit(range=range, new_text=transpiled_sql)]
return TranspileDocumentResult(uri=params.uri, changes=changes, diagnostics=diagnostics)

def _transpile(self, file_name: str, lsp_range: Range, source_sql: str) -> tuple[str, list[Diagnostic]]:
if file_name == "no_transpile.sql":
diagnostic = Diagnostic(
range=lsp_range,
message="No transpilation required",
severity=DiagnosticSeverity.Information,
code="GENERATION-NOT_REQUIRED",
)
]
return TranspileDocumentResult(uri=params.uri, changes=changes, diagnostics=[])
return source_sql, [diagnostic]
elif file_name == "unsupported_lca.sql":
diagnostic = Diagnostic(
range=lsp_range,
message="LCA conversion not supported",
severity=DiagnosticSeverity.Error,
code="ANALYSIS-UNSUPPORTED_LCA",
)
return source_sql, [diagnostic]
elif file_name == "internal.sql":
diagnostic = Diagnostic(
range=lsp_range,
message="Something went wrong",
severity=DiagnosticSeverity.Warning,
code="SOME_ERROR_CODE",
)
return source_sql, [diagnostic]
else:
# general test case
return source_sql.upper(), []


server = TestLspServer("test-lsp-server", "v0.1")
Expand Down
1 change: 1 addition & 0 deletions tests/resources/lsp_transpiler/no_transpile.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
create table stuff(name varchar(12))
1 change: 1 addition & 0 deletions tests/resources/lsp_transpiler/unsupported_lca.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
create table stuff(name varchar(12))
Loading
Loading