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

Новая диагностика "Зарезервированные имена параметров" #3187

Merged
merged 11 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
18 changes: 18 additions & 0 deletions docs/diagnostics/ReservedParameterNames.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Зарезервированные имена параметра (ReservedParameterNames)

<!-- Блоки выше заполняются автоматически, не трогать -->
## Описание диагностики
theshadowco marked this conversation as resolved.
Show resolved Hide resolved
Если имя параметра совпадает с именем системного перечисления, то в локальном контексте доступ к значениям этого перечисления будет недоступен.
theshadowco marked this conversation as resolved.
Show resolved Hide resolved
Синтаксическая проверка кода модуля не выявит такую ошибку. Чтобы предотвратить эту ситуацию имя параметра не должно совпадать с именами системных перечислений.
Список зарезервированных слов задается регулярным выражением.
Поиск производится без учета регистра символов.

**Примеры настройки:**

"ВидГруппыФормы|ВидПоляФормы"

## Источники
<!-- Необходимо указывать ссылки на все источники, из которых почерпнута информация для создания диагностики -->

* Источник: [Стандарт: Параметры процедур и функций](https://its.1c.ru/db/v8std/content/640/hdoc)
* Источник: [Стандарт: Правила образования имен переменных](https://its.1c.ru/db/v8std#content:454:hdoc)
22 changes: 22 additions & 0 deletions docs/en/diagnostics/ReservedParameterNames.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Reserved parameter names (ReservedParameterNames)

<!-- Блоки выше заполняются автоматически, не трогать -->
## Description


theshadowco marked this conversation as resolved.
Show resolved Hide resolved
If a parameter name matches one of a system enumeration's name, then all values of that enumeration will not be available in the local context.
Module code's syntax checking will not detect an error. To prevent this situation, a parameter name should not match all names of system enumerations.

Parameter names should not contain reserved words such as system enumerations.
The list of reserved words is set by a regular expression.
The search is case-insensitive.

**For example:**

"FormGroupType|FormFieldType"

## Sources
<!-- Необходимо указывать ссылки на все источники, из которых почерпнута информация для создания диагностики -->

* Source: [Standard: Procedure and Function Parameters (RU)](https://its.1c.ru/db/v8std/content/640/hdoc)
* Source: [Standard: Rules for generating variable names (RU)](https://its.1c.ru/db/v8std#content:454:hdoc)
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
* This file is a part of BSL Language Server.
*
* Copyright (c) 2018-2023
* Alexey Sosnoviy <[email protected]>, Nikita Fedkin <[email protected]> and contributors
*
* SPDX-License-Identifier: LGPL-3.0-or-later
*
* BSL Language Server is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3.0 of the License, or (at your option) any later version.
*
* BSL Language Server is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with BSL Language Server.
*/
package com.github._1c_syntax.bsl.languageserver.diagnostics;

import com.github._1c_syntax.bsl.languageserver.context.symbol.MethodSymbol;
import com.github._1c_syntax.bsl.languageserver.context.symbol.ParameterDefinition;
import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticMetadata;
import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticParameter;
import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticSeverity;
import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticTag;
import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticType;

import com.github._1c_syntax.utils.CaseInsensitivePattern;

import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;

@DiagnosticMetadata(
type = DiagnosticType.CODE_SMELL,
nixel2007 marked this conversation as resolved.
Show resolved Hide resolved
severity = DiagnosticSeverity.MAJOR,
minutesToFix = 5,
tags = {
DiagnosticTag.STANDARD,
DiagnosticTag.BADPRACTICE
}
)
public class ReservedParameterNamesDiagnostic extends AbstractSymbolTreeDiagnostic {

private static final String RESERVED_WORDS_DEFAULT = "";

@DiagnosticParameter(type = String.class)
private Pattern reservedWords = CaseInsensitivePattern.compile(RESERVED_WORDS_DEFAULT);
theshadowco marked this conversation as resolved.
Show resolved Hide resolved

@Override
public void configure(Map<String, Object> configuration) {

var incomingMask = (String) configuration.getOrDefault("reservedWords", RESERVED_WORDS_DEFAULT);

this.reservedWords = CaseInsensitivePattern.compile("^" + incomingMask.trim() + "$");
}
Comment on lines +54 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

Добавьте проверку на пустую строку.

Метод настройки должен дополнительно проверять, что шаблон зарезервированных слов не пуст.

-    this.reservedWords = CaseInsensitivePattern.compile("^" + incomingMask.trim() + "$");
+    if (!incomingMask.trim().isEmpty()) {
+      this.reservedWords = CaseInsensitivePattern.compile("^" + incomingMask.trim() + "$");
+    }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Override
public void configure(Map<String, Object> configuration) {
var incomingMask = (String) configuration.getOrDefault("reservedWords", RESERVED_WORDS_DEFAULT);
this.reservedWords = CaseInsensitivePattern.compile("^" + incomingMask.trim() + "$");
}
@Override
public void configure(Map<String, Object> configuration) {
var incomingMask = (String) configuration.getOrDefault("reservedWords", RESERVED_WORDS_DEFAULT);
if (!incomingMask.trim().isEmpty()) {
this.reservedWords = CaseInsensitivePattern.compile("^" + incomingMask.trim() + "$");
}
}


@Override
protected void check() {

if (reservedWords.pattern().isBlank()) {
theshadowco marked this conversation as resolved.
Show resolved Hide resolved
return;
}
super.check();
}

@Override
public void visitMethod(MethodSymbol methodSymbol) {

List<ParameterDefinition> parameters = methodSymbol.getParameters();
checkParameterName(parameters);
}

private void checkParameterName(List<ParameterDefinition> parameters) {

parameters.forEach((ParameterDefinition parameter) -> {

var matcher = reservedWords.matcher(parameter.getName());
if (matcher.find()) {
diagnosticStorage.addDiagnostic(parameter.getRange(), info.getMessage(parameter.getName()));
}
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1578,6 +1578,24 @@
"title": "Overuse \"Reference\" in a query",
"$id": "#/definitions/RefOveruse"
},
"ReservedParameterNames": {
"description": "Reserved parameter names",
"default": false,
"type": [
"boolean",
"object"
],
"title": "Reserved parameter names",
"properties": {
"reservedWords": {
"description": "Regular expression for reserved parameter names.",
"default": "",
"type": "string",
"title": "Regular expression for reserved parameter names."
}
},
"$id": "#/definitions/ReservedParameterNames"
},
"RewriteMethodParameter": {
"description": "Rewrite method parameter",
"default": true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,9 @@
"RefOveruse": {
"$ref": "parameters-schema.json#/definitions/RefOveruse"
},
"ReservedParameterNames": {
"$ref": "parameters-schema.json#/definitions/ReservedParameterNames"
},
"RewriteMethodParameter": {
"$ref": "parameters-schema.json#/definitions/RewriteMethodParameter"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
diagnosticMessage=Rename parameter "%s" that matches one of reserved words.
diagnosticName=Reserved parameter names
reservedWords=Regular expression for reserved parameter names.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
diagnosticMessage=Переименуйте параметр "%s" чтобы он не совпадал с зарезервированным словом.
theshadowco marked this conversation as resolved.
Show resolved Hide resolved
diagnosticName=Зарезервированные имена параметров
reservedWords=Регулярное выражение для зарезервированных имен параметров.
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* This file is a part of BSL Language Server.
*
* Copyright (c) 2018-2023
* Alexey Sosnoviy <[email protected]>, Nikita Fedkin <[email protected]> and contributors
*
* SPDX-License-Identifier: LGPL-3.0-or-later
*
* BSL Language Server is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3.0 of the License, or (at your option) any later version.
*
* BSL Language Server is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with BSL Language Server.
*/
package com.github._1c_syntax.bsl.languageserver.diagnostics;

import org.eclipse.lsp4j.Diagnostic;
import org.junit.jupiter.api.Test;

import java.util.List;
import java.util.Map;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import static com.github._1c_syntax.bsl.languageserver.util.Assertions.assertThat;

class ReservedParameterNamesDiagnosticTest extends AbstractDiagnosticTest<ReservedParameterNamesDiagnostic> {
ReservedParameterNamesDiagnosticTest() {
super(ReservedParameterNamesDiagnostic.class);
}

@Test
void testEmpty() {

List<Diagnostic> diagnostics = getDiagnostics();
assertThat(diagnostics).isEmpty();

}

@Test
void testPositive() {

Map<String, Object> configuration = diagnosticInstance.info.getDefaultConfiguration();
configuration.put("reservedWords", "ВидГруппыФормы");
diagnosticInstance.configure(configuration);

List<Diagnostic> diagnostics = getDiagnostics();

assertThat(diagnostics).hasSize(1);
assertThat(diagnostics, true)
theshadowco marked this conversation as resolved.
Show resolved Hide resolved
.hasMessageOnRange("Переименуйте параметр \"ВидГруппыФормы\" чтобы он не совпадал с зарезервированным словом.",
2, 16, 30);

}

@ParameterizedTest
@ValueSource(strings = {" ", "ВидГруппы", "ВидГруппыФормыРасширенный"})
void testNegative(String testWord) {

Map<String, Object> configuration = diagnosticInstance.info.getDefaultConfiguration();
configuration.put("reservedWords", testWord);
diagnosticInstance.configure(configuration);

List<Diagnostic> diagnostics = getDiagnostics();
assertThat(diagnostics).isEmpty();

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// при наличии в списке зарезервированного имени перечисления ВидГруппыФормы должен сработать тест

Процедура Тест1(ВидГруппыФормы) // здесь должен сработать тест

А = ВидГруппыФормы.ОбычнаяГруппа;

КонецПроцедуры
Loading