Skip to content

#1329 Исправлена ошибка в проверке manager-module-named-self-reference #1336

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.

Sign up for GitHub

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

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

#1329 Исправлена ошибка в проверке manager-module-named-self-reference #1336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dfcdd12
#1329 Исправлена ошибка в проверке manager-module-named-self-reference
VAGoncharov Jun 12, 2023
1f0aac6
#1329 Исправлено формирование
VAGoncharov Jun 12, 2023
c451559
#1329 Поправлены комментарии
VAGoncharov Jun 12, 2023
5932bcb
#1329 Merge remote-tracking branch 'upstream/master' into feature/132…
VAGoncharov Jun 19, 2023
9c84501
Merge remote-tracking branch 'upstream/master' into
VAGoncharov Jun 19, 2023
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
94 changes: 78 additions & 16 deletions ...codestyle.bsl/src/com/e1c/v8codestyle/bsl/check/ManagerModuleNamedSelfReferenceCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (C) 2022, 1C-Soft LLC and others.
* Copyright (C) 2023, 1C-Soft LLC and others.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
Expand All @@ -14,44 +14,85 @@

import static com._1c.g5.v8.dt.bsl.model.BslPackage.Literals.DYNAMIC_FEATURE_ACCESS;

import java.util.Map;

import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.emf.ecore.EClass;
import org.eclipse.emf.ecore.EObject;
import org.eclipse.emf.ecore.EReference;
import org.eclipse.xtext.EcoreUtil2;
import org.eclipse.xtext.naming.IQualifiedNameProvider;
import org.eclipse.xtext.naming.IQualifiedNameConverter;
import org.eclipse.xtext.resource.IEObjectDescription;
import org.eclipse.xtext.scoping.IScope;
import org.eclipse.xtext.scoping.IScopeProvider;

import com._1c.g5.v8.dt.bsl.documentation.comment.LinkPart;
import com._1c.g5.v8.dt.bsl.model.DynamicFeatureAccess;
import com._1c.g5.v8.dt.bsl.model.Expression;
import com._1c.g5.v8.dt.bsl.model.Module;
import com._1c.g5.v8.dt.bsl.model.ModuleType;
import com._1c.g5.v8.dt.bsl.model.StaticFeatureAccess;
import com._1c.g5.v8.dt.common.StringUtils;
import com._1c.g5.v8.dt.core.naming.ITopObjectFqnGenerator;
import com._1c.g5.v8.dt.lcore.util.CaseInsensitiveString;
import com._1c.g5.v8.dt.metadata.mdclass.MdClassPackage;
import com.e1c.g5.v8.dt.check.CheckComplexity;
import com.e1c.g5.v8.dt.check.ICheckParameters;
import com.e1c.g5.v8.dt.check.components.BasicCheck;
import com.e1c.g5.v8.dt.check.settings.IssueSeverity;
import com.e1c.g5.v8.dt.check.settings.IssueType;
import com.e1c.v8codestyle.check.StandardCheckExtension;
import com.e1c.v8codestyle.internal.bsl.BslPlugin;
import com.google.common.collect.ImmutableMap;
import com.google.inject.Inject;

/**
* CHeck self reference by name in manager modules.
* Check self reference by name in manager modules.
*
* @author Maxim Galios
* @author Vadim Goncharov
*
*/
public class ManagerModuleNamedSelfReferenceCheck
extends BasicCheck
{
private static final String CHECK_ID = "manager-module-named-self-reference"; //$NON-NLS-1$

private final IQualifiedNameProvider bslQualifiedNameProvider;
private final IScopeProvider scopeProvider;

private final IQualifiedNameConverter qualifiedNameConverter;

private final ITopObjectFqnGenerator topObjectFqnGenerator;

private static final Map<EReference, EClass> MANAGERS_FOR_MDOBJECT = new ImmutableMap.Builder<EReference, EClass>()
.put(MdClassPackage.Literals.CONFIGURATION__CATALOGS, MdClassPackage.Literals.CATALOG)
.put(MdClassPackage.Literals.CONFIGURATION__DOCUMENTS, MdClassPackage.Literals.DOCUMENT)
.put(MdClassPackage.Literals.CONFIGURATION__DOCUMENT_JOURNALS, MdClassPackage.Literals.DOCUMENT_JOURNAL)
.put(MdClassPackage.Literals.CONFIGURATION__ENUMS, MdClassPackage.Literals.ENUM)
.put(MdClassPackage.Literals.CONFIGURATION__REPORTS, MdClassPackage.Literals.REPORT)
.put(MdClassPackage.Literals.CONFIGURATION__DATA_PROCESSORS, MdClassPackage.Literals.DATA_PROCESSOR)
.put(MdClassPackage.Literals.CONFIGURATION__CHARTS_OF_CHARACTERISTIC_TYPES,
MdClassPackage.Literals.CHART_OF_CHARACTERISTIC_TYPES)
.put(MdClassPackage.Literals.CONFIGURATION__CHARTS_OF_ACCOUNTS, MdClassPackage.Literals.CHART_OF_ACCOUNTS)
.put(MdClassPackage.Literals.CONFIGURATION__CHARTS_OF_CALCULATION_TYPES,
MdClassPackage.Literals.CHART_OF_CALCULATION_TYPES)
.put(MdClassPackage.Literals.CONFIGURATION__INFORMATION_REGISTERS, MdClassPackage.Literals.INFORMATION_REGISTER)
.put(MdClassPackage.Literals.CONFIGURATION__ACCUMULATION_REGISTERS,
MdClassPackage.Literals.ACCUMULATION_REGISTER)
.put(MdClassPackage.Literals.CONFIGURATION__ACCOUNTING_REGISTERS, MdClassPackage.Literals.ACCOUNTING_REGISTER)
.put(MdClassPackage.Literals.CONFIGURATION__CALCULATION_REGISTERS, MdClassPackage.Literals.CALCULATION_REGISTER)
.put(MdClassPackage.Literals.CONFIGURATION__BUSINESS_PROCESSES, MdClassPackage.Literals.BUSINESS_PROCESS)
.put(MdClassPackage.Literals.CONFIGURATION__TASKS, MdClassPackage.Literals.TASK)
.put(MdClassPackage.Literals.CONFIGURATION__EXCHANGE_PLANS, MdClassPackage.Literals.EXCHANGE_PLAN)
.put(MdClassPackage.Literals.CONFIGURATION__EXTERNAL_DATA_SOURCES, MdClassPackage.Literals.EXTERNAL_DATA_SOURCE)
.build();

@Inject
public ManagerModuleNamedSelfReferenceCheck(IQualifiedNameProvider bslQualifiedNameProvider)
public ManagerModuleNamedSelfReferenceCheck(IScopeProvider scopeProvider,
IQualifiedNameConverter qualifiedNameConverter, ITopObjectFqnGenerator topObjectFqnGenerator)
{
this.bslQualifiedNameProvider = bslQualifiedNameProvider;
this.scopeProvider = scopeProvider;
this.qualifiedNameConverter = qualifiedNameConverter;
this.topObjectFqnGenerator = topObjectFqnGenerator;
}

@Override
Expand All @@ -69,6 +110,7 @@ protected void configureCheck(CheckConfigurer builder)
.severity(IssueSeverity.MINOR)
.issueType(IssueType.CODE_STYLE)
.extension(new StandardCheckExtension(467, getCheckId(), BslPlugin.PLUGIN_ID))
.extension(ModuleTypeFilter.onlyTypes(ModuleType.MANAGER_MODULE))
.module()
.checkedObjectType(DYNAMIC_FEATURE_ACCESS);
}
Expand All @@ -80,8 +122,7 @@ protected void check(Object object, ResultAcceptor resultAceptor, ICheckParamete
Expression featureAccessSource = ((DynamicFeatureAccess)object).getSource();
Module module = EcoreUtil2.getContainerOfType(featureAccessSource, Module.class);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Модуль не нужно получать если дальше не используется


if (monitor.isCanceled() || !(featureAccessSource instanceof DynamicFeatureAccess)
|| module.getModuleType() != ModuleType.MANAGER_MODULE)
if (monitor.isCanceled() || !(featureAccessSource instanceof DynamicFeatureAccess))
{
return;
}
Expand All @@ -96,22 +137,43 @@ protected void check(Object object, ResultAcceptor resultAceptor, ICheckParamete

StaticFeatureAccess managerType = (StaticFeatureAccess)managerTypeExpression;

if (isManagerTypeValid(managerType) && isReferenceExcessive(source, module))
EReference eRef = getConfigurationMdObjectRef(managerType);
if (monitor.isCanceled() || eRef == null)
{
return;
}

IEObjectDescription objectDesc = getObjectFromScope(source, eRef);
if (monitor.isCanceled() || objectDesc == null)
{
return;
}

EObject eObject = objectDesc.getEObjectOrProxy();
if (!monitor.isCanceled() && eObject.equals(module.getOwner()))
Comment on lines +152 to +153
Copy link
Collaborator

Choose a reason for hiding this comment

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

Зачитывать 2 объекта метаданных - только чтобы сравнить их полные имена - это очень дорого.
Предлагаю способ дешевле!

{
resultAceptor.addIssue(Messages.ManagerModuleNamedSelfReferenceCheck_issue, source);
}
}

private boolean isManagerTypeValid(StaticFeatureAccess managerType)
private EReference getConfigurationMdObjectRef(StaticFeatureAccess managerType)
{
EReference result = null;
CaseInsensitiveString managerTypeName = new CaseInsensitiveString(managerType.getName());
return LinkPart.MD_OBJECT_MANAGERS.containsKey(managerTypeName)
|| LinkPart.MD_OBJECT_MANAGERS_RU.containsKey(managerTypeName);
result = LinkPart.MD_OBJECT_MANAGERS.get(managerTypeName);
if (result == null)
{
result = LinkPart.MD_OBJECT_MANAGERS_RU.get(managerTypeName);
}
return result;
}

private boolean isReferenceExcessive(DynamicFeatureAccess source, Module module)
private IEObjectDescription getObjectFromScope(DynamicFeatureAccess source, EReference reference)
{
return StringUtils.equals(bslQualifiedNameProvider.getFullyQualifiedName(module).getSegment(1),
source.getName());
IScope scope = scopeProvider.getScope(source, reference);
String fqn =
topObjectFqnGenerator.generateStandaloneObjectFqn(MANAGERS_FOR_MDOBJECT.get(reference), source.getName());
return scope.getSingleElement(qualifiedNameConverter.toQualifiedName(fqn));
Comment on lines -114 to +176
Copy link
Collaborator

Choose a reason for hiding this comment

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

Тут нет смысла генерить FQN для целевого объекта, а еще получать сам объект из скоупа - это дорогостоящее!

Достаточно проверить URI для текущего объекта (модуль не нужно получать выше!) с именем ссылки:

private boolean isReferenceExcessive(DynamicFeatureAccess source, , EReference reference)
{
URI uri = EcoreUtil.getURI(source).trimFragment(); // тут будет URI текущего модуля
return uri.segmentCount()==5 && source.getName() != null && source.getName().equals(uri.segment(3)) && reference.getName().equals(uri.segment(2));
}

reference.getName() - всегда английское, и имя каталога для типа менеджера - тоже всегда будет английским.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Можно так же второй способ - более тяжелый т.к. получает лишний объект, но выглядящий менее хакирующе:

private boolean isReferenceExcessive(DynamicFeatureAccess source, Module module, EReference reference)
{
EObject owner = module.getOwner();
return owner instanceof MdObject && source.getName() != null && source.getName().equals(((MdObject)owner).getName) && reference.getEReferenceType().equals(owner.eClass());
}

}

}
4 changes: 2 additions & 2 deletions ...ts/src/com/e1c/v8codestyle/bsl/check/itests/ManagerModuleNamedSelfReferenceCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (C) 2022, 1C-Soft LLC and others.
* Copyright (C) 2023, 1C-Soft LLC and others.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -34,7 +34,7 @@ public class ManagerModuleNamedSelfReferenceCheckTest
{
private static final String PROJECT_NAME = "ManagerModuleNamedSelfReferenceCheck";

private static final String MANAGER_MODULE_FILE_NAME = "/src/Catalogs/Catalog/ManagerModule.bsl";
private static final String MANAGER_MODULE_FILE_NAME = "/src/Catalogs/ProductionStatus/ManagerModule.bsl";

public ManagerModuleNamedSelfReferenceCheckTest()
{
Expand Down
8 changes: 0 additions & 8 deletions ...sts/workspace/ManagerModuleNamedSelfReferenceCheck/src/Catalogs/Catalog/ManagerModule.bsl
View file
Open in desktop

This file was deleted.

10 changes: 10 additions & 0 deletions ...pace/ManagerModuleNamedSelfReferenceCheck/src/Catalogs/ProductionStatus/ManagerModule.bsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Функция МояФункция() экспорт
Возврат 1;
КонецФункции

Процедура Тест()
Catalogs.ProductionStatus.МойРеквизит = Catalogs.ProductionStatus.МояФункция();
Справочники.ProductionStatus.МойРеквизит = Справочники.ProductionStatus.МояФункция();
Переменная1 = Enums.ProductionStatus.EmptyRef();
Переменная1 = Перечисления.ProductionStatus.ПустаяСсылка();
КонецПроцедуры
8 changes: 4 additions & 4 deletions ...nceCheck/src/Catalogs/Catalog/Catalog.mdo → ...ogs/ProductionStatus/ProductionStatus.mdo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
<listType typeId="8720b617-2689-4d03-bb9a-ba7bf139c16e" valueTypeId="1feedc4e-0dc9-4b58-bd01-c3faab1d5529"/>
<managerType typeId="2544c98a-3054-4404-ae06-aa637b707cc5" valueTypeId="62031592-69bd-4694-87cb-0c9edebb5026"/>
</producedTypes>
<name>Catalog</name>
<name>ProductionStatus</name>
<synonym>
<key>en</key>
<value>Catalog</value>
<value>Production status</value>
</synonym>
<useStandardCommands>true</useStandardCommands>
<inputByString>Catalog.Catalog.StandardAttribute.Code</inputByString>
<inputByString>Catalog.Catalog.StandardAttribute.Description</inputByString>
<inputByString>Catalog.ProductionStatus.StandardAttribute.Code</inputByString>
<inputByString>Catalog.ProductionStatus.StandardAttribute.Description</inputByString>
<fullTextSearchOnInputByString>DontUse</fullTextSearchOnInputByString>
<createOnInput>Use</createOnInput>
<dataLockControlMode>Managed</dataLockControlMode>
Expand Down
3 changes: 2 additions & 1 deletion ...itests/workspace/ManagerModuleNamedSelfReferenceCheck/src/Configuration/Configuration.mdo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,6 @@
</synonym>
<languageCode>en</languageCode>
</languages>
<catalogs>Catalog.Catalog</catalogs>
<catalogs>Catalog.ProductionStatus</catalogs>
<enums>Enum.ProductionStatus</enums>
</mdclass:Configuration>
15 changes: 15 additions & 0 deletions ...pace/ManagerModuleNamedSelfReferenceCheck/src/Enums/ProductionStatus/ProductionStatus.mdo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version="1.0" encoding="UTF-8"?>
<mdclass:Enum xmlns:mdclass="http://g5.1c.ru/v8/dt/metadata/mdclass" uuid="665b6d12-b6ae-4f5d-97f8-3c96f47aba3a">
<producedTypes>
<refType typeId="d4f3f7c9-f164-44c2-8823-9bf81ba72c86" valueTypeId="52d61ea9-b467-4145-be4b-d605c5c97049"/>
<listType typeId="2bfc1ccc-f7e5-4ea2-88a5-295e26af6178" valueTypeId="3e79fee9-60e2-4636-a687-a06a883a6318"/>
<managerType typeId="c8de2bce-15ed-4ac0-b991-c9b6696ee190" valueTypeId="07d673a8-3fb9-41dc-881e-0360b8b18462"/>
</producedTypes>
<name>ProductionStatus</name>
<synonym>
<key>en</key>
<value>Production status</value>
</synonym>
<quickChoice>true</quickChoice>
<choiceMode>BothWays</choiceMode>
</mdclass:Enum>