Skip to content

Commit

Permalink
Merge pull request #1232 from alkoleft/feature/exportVariables
Browse files Browse the repository at this point in the history
Feature/export variables
  • Loading branch information
nixel2007 authored Jun 9, 2020
2 parents 4d918b6 + 48bc68e commit f37f584
Show file tree
Hide file tree
Showing 17 changed files with 383 additions and 4 deletions.
65 changes: 65 additions & 0 deletions docs/diagnostics/ExportVariables.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# Запрет экспортных глобальных переменных модуля (ExportVariables)

| Тип | Поддерживаются<br/>языки | Важность | Включена<br/>по умолчанию | Время на<br/>исправление (мин) | Тэги |
| :-: | :-: | :-: | :-: | :-: | :-: |
| `Дефект кода` | `BSL`<br/>`OS` | `Важный` | `Да` | `5` | `standard`<br/>`design`<br/>`unpredictable` |

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

В большинстве случаев, вместо переменных программных модулей следует использовать более подходящие средства разработки платформы 1С:Предприятие.
Поскольку область видимости (использования) таких переменных сложно контролировать,
то они зачастую становятся источником трудновоспроизводимых ошибок.

## Примеры
<!-- В данном разделе приводятся примеры, на которые диагностика срабатывает, а также можно привести пример, как можно исправить ситуацию -->

```bsl
Перем КонвертацияФайлов Экспорт;
Процедура ПередЗаписью(Отказ)
Если КонвертацияФайлов Тогда
...
КонецПроцедуры
```

Для передачи параметров между обработчиками подписок на события и в обработчики событий модуля объекта из внешнего кода
рекомендуется использовать свойство объекта ДополнительныеСвойства

```bsl
Процедура ПередЗаписью(Отказ)
Если ДополнительныеСвойства.Свойство("КонвертацияФайлов") Тогда
...
КонецПроцедуры
// вызывающий код
ФайлОбъект.ДополнительныеСвойства.Вставить("КонвертацияФайлов", Истина);
ФайлОбъект.Записать();
```

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

[Стандарт: Использование переменных в программных модулях](https://its.1c.ru/db/v8std#content:639:hdoc)

## Сниппеты

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

```bsl
// BSLLS:ExportVariables-off
// BSLLS:ExportVariables-on
```

### Параметр конфигурационного файла

```json
"ExportVariables": false
```
5 changes: 3 additions & 2 deletions docs/diagnostics/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

## Список реализованных диагностик

Общее количество: **112**
Общее количество: **113**

* Дефект кода: **70**
* Дефект кода: **71**
* Уязвимость: **3**
* Ошибка: **35**
* Потенциальная уязвимость: **4**
Expand Down Expand Up @@ -57,6 +57,7 @@
| [ExcessiveAutoTestCheck](ExcessiveAutoTestCheck.md) | Избыточная проверка параметра АвтоТест | Да | Незначительный | Дефект кода | `standard`<br/>`deprecated` |
| [ExecuteExternalCode](ExecuteExternalCode.md) | Выполнение произвольного кода на сервере | Да | Критичный | Уязвимость | `error`<br/>`standard` |
| [ExecuteExternalCodeInCommonModule](ExecuteExternalCodeInCommonModule.md) | Выполнение произвольного кода в общем модуле на сервере | Да | Критичный | Потенциальная уязвимость | `badpractice`<br/>`standard` |
| [ExportVariables](ExportVariables.md) | Запрет экспортных глобальных переменных модуля | Да | Важный | Дефект кода | `standard`<br/>`design`<br/>`unpredictable` |
| [ExtraCommas](ExtraCommas.md) | Запятые без указания параметра в конце вызова метода | Да | Важный | Дефект кода | `standard`<br/>`badpractice` |
| [FormDataToValue](FormDataToValue.md) | Использование метода ДанныеФормыВЗначение | Да | Информационный | Дефект кода | `badpractice` |
| [FunctionNameStartsWithGet](FunctionNameStartsWithGet.md) | Имя функции не должно начинаться с "Получить" | Нет | Информационный | Дефект кода | `standard` |
Expand Down
64 changes: 64 additions & 0 deletions docs/en/diagnostics/ExportVariables.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Ban export global module variables (ExportVariables)

| Type | Scope | Severity | Activated<br/>by default | Minutes<br/>to fix | Tags |
| :-: | :-: | :-: | :-: | :-: | :-: |
| `Code smell` | `BSL`<br/>`OS` | `Major` | `Yes` | `5` | `standard`<br/>`design`<br/>`unpredictable` |

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

In most scenarios, we recommend that you do not use global variables and use other 1C:Enterprise script tools instead.
Since monitoring the visibility (usage) areas of such variables is tricky,
they often might cause issues that cannot be easily located.

## Examples
<!-- В данном разделе приводятся примеры, на которые диагностика срабатывает, а также можно привести пример, как можно исправить ситуацию -->

```bsl
Variable FileConversion Export;
Procedure BeforeWrite(Cancel)
If FileConversion Then
...
EndProcedure
```

We recommend that you use the AdditionalProperties object property for passing parameters between event subscription handlers
and for passing parameters from external script to object module event handlers.

```bsl
Procedure BeforeWrite(Cancel)
If AdditionalProperties.Property("FileConversion") Then
...
EndProcedure
// script that calls the procedure
FileObject.AdditionalProperties.Insert("FileConversion", True);
FileObject.Write();
```

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

* [Standard: Using global variables in modules](https://1c-dn.com/library/using_global_variables_in_modules/)
* [Standard: Using global variables in modules(RU)](https://its.1c.ru/db/v8std#content:639:hdoc)

## Snippets

<!-- Блоки ниже заполняются автоматически, не трогать -->
### Diagnostic ignorance in code

```bsl
// BSLLS:ExportVariables-off
// BSLLS:ExportVariables-on
```

### Parameter for config

```json
"ExportVariables": false
```
5 changes: 3 additions & 2 deletions docs/en/diagnostics/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ To escape individual sections of code or files from triggering diagnostics, you

## Implemented diagnostics

Total: **112**
Total: **113**

* Error: **35**
* Code smell: **70**
* Code smell: **71**
* Vulnerability: **3**
* Security Hotspot: **4**

Expand Down Expand Up @@ -57,6 +57,7 @@ Total: **112**
| [ExcessiveAutoTestCheck](ExcessiveAutoTestCheck.md) | Excessive AutoTest Check | Yes | Minor | Code smell | `standard`<br/>`deprecated` |
| [ExecuteExternalCode](ExecuteExternalCode.md) | Executing of external code on the server | Yes | Critical | Vulnerability | `error`<br/>`standard` |
| [ExecuteExternalCodeInCommonModule](ExecuteExternalCodeInCommonModule.md) | Executing of external code in a common module on the server | Yes | Critical | Security Hotspot | `badpractice`<br/>`standard` |
| [ExportVariables](ExportVariables.md) | Ban export global module variables | Yes | Major | Code smell | `standard`<br/>`design`<br/>`unpredictable` |
| [ExtraCommas](ExtraCommas.md) | Commas without a parameter at the end of a method call | Yes | Major | Code smell | `standard`<br/>`badpractice` |
| [FormDataToValue](FormDataToValue.md) | FormDataToValue method call | Yes | Info | Code smell | `badpractice` |
| [FunctionNameStartsWithGet](FunctionNameStartsWithGet.md) | Function name shouldn't start with "Получить" | No | Info | Code smell | `standard` |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,9 @@ public Optional<RegionSymbol> getRegion() {
.filter(symbol -> symbol instanceof RegionSymbol)
.map(symbol -> (RegionSymbol) symbol);
}

@Override
public void accept(SymbolTreeVisitor visitor) {
visitor.visitMethod(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,9 @@ public List<MethodSymbol> getMethods() {
.map(symbol -> (MethodSymbol) symbol)
.collect(Collectors.toList());
}

@Override
public void accept(SymbolTreeVisitor visitor) {
visitor.visitRegion(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,15 @@ default Optional<Symbol> getRootParent() {
return getParent().flatMap(Symbol::getRootParent).or(() -> Optional.of(this));
}

void accept(SymbolTreeVisitor visitor);

static Symbol emptySymbol() {
return new Symbol() {
@Getter private final String name = "empty";
@Getter private final Range range = Ranges.create(-1, 0, -1, 0);
@Getter @Setter private Optional<Symbol> parent = Optional.empty();
@Getter private final List<Symbol> children = Collections.emptyList();
@Override public void accept(SymbolTreeVisitor visitor) { }
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* This file is a part of BSL Language Server.
*
* Copyright © 2018-2020
* Alexey Sosnoviy <labotamy@gmail.com>, Nikita Gryzlov <nixel2007@gmail.com> 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.context.symbol;

public interface SymbolTreeVisitor {
void visitRegion(RegionSymbol region);

void visitMethod(MethodSymbol method);

void visitVariable(VariableSymbol variable);
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,9 @@ public class VariableSymbol implements Symbol {
VariableKind kind;
boolean export;
Optional<VariableDescription> description;

@Override
public void accept(SymbolTreeVisitor visitor) {
visitor.visitVariable(this);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* This file is a part of BSL Language Server.
*
* Copyright © 2018-2020
* Alexey Sosnoviy <labotamy@gmail.com>, Nikita Gryzlov <nixel2007@gmail.com> 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.RegionSymbol;
import com.github._1c_syntax.bsl.languageserver.context.symbol.Symbol;
import com.github._1c_syntax.bsl.languageserver.context.symbol.SymbolTreeVisitor;
import com.github._1c_syntax.bsl.languageserver.context.symbol.VariableSymbol;
import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticInfo;

import java.util.List;

public abstract class AbstractSymbolTreeDiagnostic extends AbstractDiagnostic implements SymbolTreeVisitor {
public AbstractSymbolTreeDiagnostic(DiagnosticInfo info) {
super(info);
}

@Override
protected void check() {
visitChildren(documentContext.getSymbolTree().getChildren());
}

void visitChildren(List<Symbol> children) {
children.forEach(this::visit);
}

void visit(Symbol symbol){
symbol.accept(this);
}

@Override
public void visitRegion(RegionSymbol region) {
visitChildren(region.getChildren());
}

@Override
public void visitMethod(MethodSymbol method) {
visitChildren(method.getChildren());
}

@Override
public void visitVariable(VariableSymbol variable) {
visitChildren(variable.getChildren());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* This file is a part of BSL Language Server.
*
* Copyright © 2018-2020
* Alexey Sosnoviy <labotamy@gmail.com>, Nikita Gryzlov <nixel2007@gmail.com> 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.VariableSymbol;
import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticInfo;
import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticMetadata;
import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticScope;
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;

@DiagnosticMetadata(
type = DiagnosticType.CODE_SMELL,
severity = DiagnosticSeverity.MAJOR,
minutesToFix = 5,
scope = DiagnosticScope.ALL,
tags = {
DiagnosticTag.STANDARD,
DiagnosticTag.DESIGN,
DiagnosticTag.UNPREDICTABLE
}
)
public class ExportVariablesDiagnostic extends AbstractSymbolTreeDiagnostic {
public ExportVariablesDiagnostic(DiagnosticInfo info) {
super(info);
}

@Override
public void visitVariable(VariableSymbol variable) {
if (variable.isExport()) {
diagnosticStorage.addDiagnostic(variable.getRange());
}
}

@Override
public void visitMethod(MethodSymbol method) {
// skip content of methods
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,16 @@
"title": "Executing of external code in a common module on the server",
"$id": "#/definitions/ExecuteExternalCodeInCommonModule"
},
"ExportVariables": {
"description": "Ban export global module variables",
"default": true,
"type": [
"boolean",
"object"
],
"title": "Ban export global module variables",
"$id": "#/definitions/ExportVariables"
},
"ExtraCommas": {
"description": "Commas without a parameter at the end of a method call",
"default": true,
Expand Down
Loading

0 comments on commit f37f584

Please sign in to comment.